Bug 230814 - Thin hairline gap displayed for subpixel sized inset box-shadows
Summary: Thin hairline gap displayed for subpixel sized inset box-shadows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 14
Hardware: Mac (Intel) macOS 11
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-26 20:38 PDT by luka
Modified: 2021-10-15 17:05 PDT (History)
11 users (show)

See Also:


Attachments
Example minimal reproduction of problem (388 bytes, text/html)
2021-09-26 20:38 PDT, luka
no flags Details
New minimal reproduction of problem (274 bytes, text/html)
2021-09-26 20:51 PDT, luka
no flags Details
Patch (4.52 KB, patch)
2021-09-29 11:14 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (4.44 KB, patch)
2021-09-29 11:40 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (5.26 KB, patch)
2021-09-29 13:04 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 luka 2021-09-26 20:38:38 PDT
Created attachment 439308 [details]
Example minimal reproduction of problem

When two elements with box shadows are placed next to each other with no gap, 
and an inset box shadow is applied to each with a subpixel size, there is a thin 
hairline displayed between the box shadows in which the backgrounds of each element 
can be slightly seen.

I've attached a minimal reproduction of the problem. There are two divs, one red and 
one blue, each with an inset box shadow. Both box shadows are sized at subpixel values.
You will notice that there is a small hairline gap between the two box shadows. If you 
then zoom in with a trackpad, this gap becomes much clearer and easier to see.

This behaviour is not reproducible on Chrome/Firefox, both render these inset box 
shadows without a hairline gap.
Comment 1 luka 2021-09-26 20:51:54 PDT
Created attachment 439309 [details]
New minimal reproduction of problem
Comment 2 luka 2021-09-26 20:53:18 PDT
I've added a new/updated minimal reproduction. The box shadows don't need to be adjacent, they just have to be inset. In the new reproduction example, there is a green subpixel inset box shadow. You can see that the red background of the element can be seen to the left of the box shadow.
Comment 3 zalan 2021-09-28 20:24:01 PDT
We probably miss a pixel snapping somewhere in RenderBoxModelObject::paintBoxShadow.
Comment 4 zalan 2021-09-28 20:40:12 PDT
right, 
the ShadowStyle::Normal branch has
context.setShadow(snappedShadowOffset
while ShadowStyle::Inset
context.setShadow(shadowOffset,
Comment 5 zalan 2021-09-28 20:40:54 PDT
will upload a patch soon
Comment 6 zalan 2021-09-29 11:14:05 PDT
Created attachment 439629 [details]
Patch
Comment 7 Simon Fraser (smfr) 2021-09-29 11:18:17 PDT
Comment on attachment 439629 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=439629&action=review

> Source/WebCore/rendering/RenderBoxModelObject.cpp:2547
> +            auto snappedShadowOffset = FloatSize { roundToDevicePixel(shadowOffset.width(), deviceScaleFactor), roundToDevicePixel(shadowOffset.height(), deviceScaleFactor) };

Use `roundSizeToDevicePixels(const LayoutSize...)`
Comment 8 zalan 2021-09-29 11:40:12 PDT
Created attachment 439633 [details]
Patch
Comment 9 zalan 2021-09-29 13:04:05 PDT
Created attachment 439647 [details]
Patch
Comment 10 EWS 2021-09-29 13:38:36 PDT
Committed r283258 (242287@main): <https://commits.webkit.org/242287@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439647 [details].
Comment 11 Radar WebKit Bug Importer 2021-09-29 13:39:16 PDT
<rdar://problem/83686181>
Comment 12 Arcady Goldmints-Orlov 2021-10-15 17:05:05 PDT
Unfortunately, the test added by this change doesn't seem to pass on GTK/WPE or Win10, I guess because of the differences in rounding between the platforms. For an example: https://build.webkit.org/results/GTK-Linux-64-bit-Release-Skip-Failing-Tests/r284281%20(2055)/fast/box-shadow/hidpi-box-shadow-inset-on-subpixel-position-diffs.html