Commit b42410d0 authored by Stephen Chenney's avatar Stephen Chenney Committed by Commit Bot

Clamp sub-pixel borders accounting for zoom

A previous patch added clamping for sub-pixel borders to ensure they
did not round to zero size. The change was done in paint code, leaving
the borders as sub-pixel sized in layout. The code prior to that patch
clamped the borders during style conversion but did not account for zoom,
meaning borders disappeared under zoom.

This change removes the clamping in paint and reverts to clamping in the
style conversion code, only this time accounting for zoom when considering
whether to clamp or not. This will prevent borders growing too large as
content is made larger, while preventing sub-pixel borders from disappearing
as content is made smaller.

Design Doc: https://docs.google.com/document/d/1fAYkOFxp2Luh6OOoXxtwOehmvNRGNss58ibQtVXL0Tw/edit?usp=sharing

Bug: 763402, 834489
Change-Id: Iba2ae80b0a981736b93453c1058fd586ba10212c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097514
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750231}
parent ef4bd0b1
......@@ -52,12 +52,10 @@
#include "third_party/blink/renderer/core/css/css_uri_value.h"
#include "third_party/blink/renderer/core/css/parser/css_tokenizer.h"
#include "third_party/blink/renderer/core/css/resolver/filter_operation_resolver.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/css/resolver/transform_builder.h"
#include "third_party/blink/renderer/core/css/style_engine.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/frame/web_feature.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/style/reference_clip_path_operation.h"
#include "third_party/blink/renderer/core/style/shape_clip_path_operation.h"
#include "third_party/blink/renderer/core/style/style_svg_resource.h"
......@@ -978,8 +976,13 @@ float StyleBuilderConverter::ConvertBorderWidth(StyleResolverState& state,
return 0;
}
const auto& primitive_value = To<CSSPrimitiveValue>(value);
return primitive_value.ComputeLength<float>(
state.CssToLengthConversionData());
double result =
primitive_value.ComputeLength<float>(state.CssToLengthConversionData());
double zoomed_result = state.StyleRef().EffectiveZoom() * result;
if (zoomed_result > 0.0 && zoomed_result < 1.0)
return 1.0;
return clampTo<float>(result, defaultMinimumForClamp<float>(),
defaultMaximumForClamp<float>());
}
GapLength StyleBuilderConverter::ConvertGapLength(StyleResolverState& state,
......
......@@ -36,6 +36,8 @@
#include "third_party/blink/renderer/core/css/css_value_list.h"
#include "third_party/blink/renderer/core/css/css_value_pair.h"
#include "third_party/blink/renderer/core/css/css_variable_data.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/style/grid_area.h"
#include "third_party/blink/renderer/core/style/grid_positions_resolver.h"
#include "third_party/blink/renderer/core/style/named_grid_lines_map.h"
......@@ -321,7 +323,8 @@ T StyleBuilderConverter::ConvertLineWidth(StyleResolverState& state,
// Reference crbug.com/485650 and crbug.com/382483
double result =
primitive_value.ComputeLength<double>(CssToLengthConversionData(state));
if (result > 0.0 && result < 1.0)
double zoomed_result = state.StyleRef().EffectiveZoom() * result;
if (zoomed_result > 0.0 && zoomed_result < 1.0)
return 1.0;
return clampTo<T>(RoundForImpreciseConversion<T>(result),
defaultMinimumForClamp<T>(), defaultMaximumForClamp<T>());
......
......@@ -289,20 +289,12 @@ LayoutRectOutsets DoubleStripeInsets(const BorderEdge edges[],
stripe));
}
float ClampOrRound(float border_width) {
// Make sure non-zero borders never disappear
if (border_width > 0.0f && border_width <= 1.0f)
return 1.0f;
return roundf(border_width);
}
void DrawSolidBorderRect(GraphicsContext& context,
const FloatRect& border_rect,
float border_width,
const Color& color) {
FloatRect stroke_rect(border_rect);
border_width = ClampOrRound(border_width);
border_width = roundf(border_width);
stroke_rect.Inflate(-border_width / 2);
bool was_antialias = context.ShouldAntialias();
......@@ -854,7 +846,7 @@ void BoxBorderPainter::PaintSide(GraphicsContext& context,
if (use_path)
path = &border_info.rounded_border_path;
else
side_rect.SetHeight(ClampOrRound(edge.Width()));
side_rect.SetHeight(roundf(edge.Width()));
PaintOneBorderSide(context, side_rect, BoxSide::kTop, BoxSide::kLeft,
BoxSide::kRight, path, border_info.anti_alias, color,
......@@ -869,7 +861,7 @@ void BoxBorderPainter::PaintSide(GraphicsContext& context,
if (use_path)
path = &border_info.rounded_border_path;
else
side_rect.ShiftYEdgeTo(side_rect.MaxY() - ClampOrRound(edge.Width()));
side_rect.ShiftYEdgeTo(side_rect.MaxY() - roundf(edge.Width()));
PaintOneBorderSide(context, side_rect, BoxSide::kBottom, BoxSide::kLeft,
BoxSide::kRight, path, border_info.anti_alias, color,
......@@ -884,7 +876,7 @@ void BoxBorderPainter::PaintSide(GraphicsContext& context,
if (use_path)
path = &border_info.rounded_border_path;
else
side_rect.SetWidth(ClampOrRound(edge.Width()));
side_rect.SetWidth(roundf(edge.Width()));
PaintOneBorderSide(context, side_rect, BoxSide::kLeft, BoxSide::kTop,
BoxSide::kBottom, path, border_info.anti_alias, color,
......@@ -899,7 +891,7 @@ void BoxBorderPainter::PaintSide(GraphicsContext& context,
if (use_path)
path = &border_info.rounded_border_path;
else
side_rect.ShiftXEdgeTo(side_rect.MaxX() - ClampOrRound(edge.Width()));
side_rect.ShiftXEdgeTo(side_rect.MaxX() - roundf(edge.Width()));
PaintOneBorderSide(context, side_rect, BoxSide::kRight, BoxSide::kTop,
BoxSide::kBottom, path, border_info.anti_alias, color,
......@@ -1017,9 +1009,8 @@ void BoxBorderPainter::PaintOneBorderSide(
ObjectPainter::DrawLineForBoxSide(
graphics_context, side_rect.X(), side_rect.Y(), side_rect.MaxX(),
side_rect.MaxY(), side, color, edge_to_render.BorderStyle(),
miter1 != kNoMiter ? ClampOrRound(adjacent_edge1.Width()) : 0,
miter2 != kNoMiter ? ClampOrRound(adjacent_edge2.Width()) : 0,
antialias);
miter1 != kNoMiter ? roundf(adjacent_edge1.Width()) : 0,
miter2 != kNoMiter ? roundf(adjacent_edge2.Width()) : 0, antialias);
}
}
......
<!DOCTYPE html>
<link rel="author" title="Stephen Chenney" href="mailto:schenney@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#border-width">
<head>
<style>
.outer {
border: solid 1px black;
background: red;
width: 101px;
height: 101px;
margin: 5px;
padding: 0px;
box-sizing: border-box;
}
.inner {
background: lightgreen;
box-sizing: border-box;
padding: 0px;
}
#inner1 {
width: 99px;
height: 99px;
}
#inner2 {
width: 100%;
height: 100%;
}
</style>
</head>
<body>
<div class="outer">
<div class="inner" id="inner1"></div>
</div>
<div class="outer">
<div class="inner" id="inner2"></div>
</div>
</body>
<!DOCTYPE html>
<link rel="author" title="Stephen Chenney" href="mailto:schenney@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#border-width">
<head>
<style>
.outer {
border: solid 1px black;
background: red;
width: 100px;
height: 100px;
margin: 5px;
}
.inner {
background: lightgreen
}
#inner1 {
width: 100px;
height: 100px;
}
#inner2 {
width: 100%;
height: 100%;
}
</style>
</head>
<body>
<div class="outer">
<div class="inner" id="inner1"></div>
</div>
<div class="outer">
<div class="inner" id="inner2"></div>
</div>
</body>
<!DOCTYPE html>
<link rel="author" title="Stephen Chenney" href="mailto:schenney@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#border-width">
<link rel="match" href="reference/subpixel-borders-with-child-border-box-sizing-ref.html">
<meta name="assert" content="Sub-pixel borders should always appear, and not be
overdrawn by child content with box-sizing: border-box." />
<head>
<style>
body { }
.outer {
border: solid .5px black;
background: red;
width: 101px;
height: 101px;
margin: 5px;
padding: 0px;
box-sizing: border-box;
}
.inner {
background: lightgreen;
padding: 0px;
box-sizing: border-box;
}
#inner1 {
width: 99px;
height: 99px;
}
#inner2 {
width: 100%;
height: 100%;
}
</style>
</head>
<body>
<div class="outer">
<div class="inner" id="inner1"></div>
</div>
<div class="outer">
<div class="inner" id="inner2"></div>
</div>
</body>
<!DOCTYPE html>
<link rel="author" title="Stephen Chenney" href="mailto:schenney@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#border-width">
<link rel="match" href="reference/subpixel-borders-with-child-ref.html">
<meta name="assert" content="Sub-pixel borders should always appear, and not be
overdrawn by child content." />
<head>
<style>
.outer {
border: solid .5px black;
background: red;
width: 100px;
height: 100px;
margin: 5px;
}
.inner {
background: lightgreen
}
#inner1 {
width: 100px;
height: 100px;
}
#inner2 {
width: 100%;
height: 100%;
}
</style>
</head>
<body>
<div class="outer">
<div class="inner" id="inner1"></div>
</div>
<div class="outer">
<div class="inner" id="inner2"></div>
</div>
</body>
......@@ -6,14 +6,14 @@
div {
zoom: 0.5;
width: 140px;
width: 148px;
}
span {
width: 68px;
width: 70px;
display: inline-block;
background-color: lime;
padding: 1.0px;
padding: 2.0px;
}
</style>
......@@ -23,7 +23,7 @@ span {
<span>Shouldn't</span><span>wrap</span>
</div>
The border width should be 1px, reported as: 1px
The border width should be 2px, reported as: 2px
</body>
</html>
......@@ -6,14 +6,14 @@
div#test {
zoom: 0.5;
width: 140px;
width: 148px;
display: block;
}
span {
background-color: lime;
float: left;
width: 68px;
width: 70px;
border: 1.0px solid lime;
}
......@@ -29,7 +29,7 @@ span {
<script>
var prop = window.getComputedStyle(document.getElementById("measure")).getPropertyValue("border-width");
document.getElementById("output").innerHTML = "The border width should be 1px, reported as: " + prop;
document.getElementById("output").innerHTML = "The border width should be 2px, reported as: " + prop;
</script>
</body>
</html>
......
......@@ -19,9 +19,9 @@
</style>
</head>
<body>
<div class="test" style="width: 78.5px; height: 78.5px">&nbsp;</div>
<div class="test" style="width: 79px; height: 79px">&nbsp;</div>
<div class="test" style="width: 79.5px; height: 79.5px">&nbsp;</div>
<div class="test" style="width: 80px; height: 80px">&nbsp;</div>
<div class="test" style="width: 80px; height: 80px">&nbsp;</div>
<div class="test" style="width: 80px; height: 80px">&nbsp;</div>
<div class="test">&nbsp;</div>
<div class="test" style="width: 80.5px; height: 80.5px">&nbsp;</div>
<br>
......
......@@ -46,24 +46,24 @@
test(function() {
var refSize = testElements['0.25px'].getBoundingClientRect();
var testSize = testElements['0.5px'].getBoundingClientRect();
assert_greater_than(testSize.width, refSize.width,
'Size of 0.5px box should be larger than 0.25px box.');
assert_greater_than(testSize.height, refSize.height,
'Size of 0.5px box should be larger than 0.25px box.');
assert_equals(testSize.width, refSize.width,
'Size of 0.5px box should be equal to 0.25px box.');
assert_equals(testSize.height, refSize.height,
'Size of 0.5px box should be equal to 0.25px box.');
refSize = testSize;
testSize = testElements['0.75px'].getBoundingClientRect();
assert_greater_than(testSize.width, refSize.width,
'Size of 0.75px box should be larger than 0.5px box.');
assert_greater_than(testSize.height, refSize.height,
'Size of 0.75px box should be larger than 0.5px box.');
assert_equals(testSize.width, refSize.width,
'Size of 0.75px box should be equal to 0.5px box.');
assert_equals(testSize.height, refSize.height,
'Size of 0.75px box should be equal to 0.5px box.');
refSize = testSize;
testSize = testElements['1px'].getBoundingClientRect();
assert_greater_than(testSize.width, refSize.width,
'Size of 1px box should be larger than 0.75px box.');
assert_greater_than(testSize.height, refSize.height,
'Size of 1px box should be larger than 0.75px box.');
assert_equals(testSize.width, refSize.width,
'Size of 1px box should be equal to 0.75px box.');
assert_equals(testSize.height, refSize.height,
'Size of 1px box should be equal to 0.75px box.');
refSize = testSize;
testSize = testElements['1.25px'].getBoundingClientRect();
......@@ -82,14 +82,14 @@
test(function() {
assert_equals(borderAsString(testElements['0.25px']),
'0.25px 0.25px 0.25px 0.25px',
'Border thickness of 0.25px box should be 0.25px.');
'1px 1px 1px 1px',
'Border thickness of 0.25px box should be 1px.');
assert_equals(borderAsString(testElements['0.5px']),
'0.5px 0.5px 0.5px 0.5px',
'Border thickness of 0.5px box should be 0.5px.');
'1px 1px 1px 1px',
'Border thickness of 0.5px box should be 1px.');
assert_equals(borderAsString(testElements['0.75px']),
'0.75px 0.75px 0.75px 0.75px',
'Border thickness of 0.75px box should be 0.75px.');
'1px 1px 1px 1px',
'Border thickness of 0.75px box should be 1px.');
assert_equals(borderAsString(testElements['1.25px']),
'1.25px 1.25px 1.25px 1.25px',
'Border thickness of 1.25px box should retain decimals.');
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment