Bug 156121 - CSS Triangles Rendering Regression affecting CSS Ribbons
Summary: CSS Triangles Rendering Regression affecting CSS Ribbons
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari 9
Hardware: All All
: P2 Normal
Assignee: zalan
URL: http://www.maisonrichard.com/test
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-01 16:55 PDT by hexalys
Modified: 2016-04-04 20:50 PDT (History)
5 users (show)

See Also:


Attachments
Visual of rendering defect on CSS Ribbon (17.72 KB, image/png)
2016-04-01 16:55 PDT, hexalys
no flags Details
Test reduction (264 bytes, text/html)
2016-04-01 21:47 PDT, zalan
no flags Details
Patch (80.72 KB, patch)
2016-04-03 20:29 PDT, zalan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.47 MB, application/zip)
2016-04-03 21:21 PDT, Build Bot
no flags Details
Patch (130.36 KB, patch)
2016-04-04 08:47 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description hexalys 2016-04-01 16:55:40 PDT
Created attachment 275444 [details]
Visual of rendering defect on CSS Ribbon

A rendering regression has ben introduced since at least Safari 8+ (perhaps as early as 7) which breaks CSS triangle shapes, as shown in the snapshot. If you visit the sample url on Safari 8 or 9, you can see the ribbon at the footer of the site does not render properly in Safari 8+. All other browsers including Webkit as far back as Safari 5.1 are showing the ribbon inner edges as expected.
Comment 1 Jon Lee 2016-04-01 17:00:26 PDT
Looks like sub pixel.
Comment 2 Radar WebKit Bug Importer 2016-04-01 17:01:19 PDT
<rdar://problem/25503257>
Comment 3 Simon Fraser (smfr) 2016-04-01 17:12:00 PDT
Changed in the mid-late 160000's.

Can't reproduce in a simple reduction.
Comment 4 hexalys 2016-04-01 18:27:23 PDT
(In reply to comment #3)
> Changed in the mid-late 160000's.
> 
> Can't reproduce in a simple reduction.

I know it a peculiar issue... After fiddling with it. One pointer I can give you is this:

If you disable the 'font-size' on #footer in the example, or set it to 1em instead of 0.938em. The problem goes away. However that isn't the font size I want, and reveal that somthing is up.

It appears the font-size is changing the pseudo(s) behavior in unexpected ways.

Is that enough info for you the assess the cause and pinpoint what was changed?
Comment 5 hexalys 2016-04-01 21:13:29 PDT
I have put together a minimal repro.
http://goo.gl/RtOLiY
Comment 6 zalan 2016-04-01 21:47:10 PDT
Created attachment 275461 [details]
Test reduction
Comment 7 zalan 2016-04-01 21:47:30 PDT
(In reply to comment #5)
> I have put together a minimal repro.
> http://goo.gl/RtOLiY

Thanks for the reduction!
Comment 8 zalan 2016-04-02 22:15:27 PDT
This fixes the painting issue and matches FF's border sizing behavior (floor border width to device pixel).
 
diff --git a/Source/WebCore/css/StyleBuilderConverter.h b/Source/WebCore/css/StyleBuilderConverter.h
index ef6b19d..f9da9da 100644
--- a/Source/WebCore/css/StyleBuilderConverter.h
+++ b/Source/WebCore/css/StyleBuilderConverter.h
@@ -40,6 +40,7 @@
 #include "CSSPrimitiveValue.h"
 #include "CSSReflectValue.h"
 #include "Frame.h"
+#include "LayoutUnit.h"
 #include "Length.h"
 #include "LengthRepeat.h"
 #include "Pair.h"
@@ -254,7 +255,7 @@ inline T StyleBuilderConverter::convertLineWidth(StyleResolver& styleResolver, C
         float minimumLineWidth = 1 / styleResolver.document().deviceScaleFactor();
         if (result > 0 && result < minimumLineWidth)
             return minimumLineWidth;
-        return result;
+        return floorToDevicePixel(result, styleResolver.document().deviceScaleFactor());
     }
     default:
         ASSERT_NOT_REACHED();

- I am expecting tons of rebaseline on this :(
Comment 9 zalan 2016-04-03 17:21:53 PDT
with this change getComputedStyle() reports the floored border width value, but that's also inline with what FF reports.
Comment 10 zalan 2016-04-03 20:29:51 PDT
Created attachment 275527 [details]
Patch
Comment 11 Build Bot 2016-04-03 21:21:44 PDT
Comment on attachment 275527 [details]
Patch

Attachment 275527 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1095688

New failing tests:
fast/block/float/016.html
ietestcenter/css3/bordersbackgrounds/border-radius-style-002.htm
fast/css/bidi-override-in-anonymous-block.html
ietestcenter/css3/bordersbackgrounds/border-radius-style-001.htm
ietestcenter/css3/bordersbackgrounds/border-top-left-radius-values-003.htm
tables/mozilla_expected_failures/bugs/bug1055-2.html
ietestcenter/css3/bordersbackgrounds/border-radius-with-three-values-001.htm
ietestcenter/css3/bordersbackgrounds/border-radius-style-004.htm
css1/units/length_units.html
ietestcenter/css3/bordersbackgrounds/border-radius-with-two-values-001.htm
ietestcenter/css3/bordersbackgrounds/border-radius-initial-value-001.htm
Comment 12 Build Bot 2016-04-03 21:21:47 PDT
Created attachment 275528 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5
Comment 13 zalan 2016-04-04 08:47:27 PDT
Created attachment 275549 [details]
Patch
Comment 14 WebKit Commit Bot 2016-04-04 20:50:49 PDT
Comment on attachment 275549 [details]
Patch

Clearing flags on attachment: 275549

Committed r199034: <http://trac.webkit.org/changeset/199034>
Comment 15 WebKit Commit Bot 2016-04-04 20:50:54 PDT
All reviewed patches have been landed.  Closing bug.