1227216 - Strange inset shadow in FF 45.0a1 (2015-11-22)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Sergei Vasiliev]

Attachment: ff-shadow-issue.png

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151122004109

Steps to reproduce:

1. Create first element with `box-shadow: inset 0 0 1px -3px rgba(0,0,10,0.25)`.
2. Second one will got `box-shadow: inset 0 5px 1px -2px rgba(0,0,10,0.25)`

Example: http://jsfiddle.net/SerGen/uv6r12hy/


Actual results:

1. Shadow covers all element
2. Offset clears shadow


Expected results:

1, 2. Shadow must appear only on top of element with little blur.

Comment 1

[Loic]

Attachment: screenshot-shadow-ff42-45.png

I can't reproduce the issue with the 1st square but there is a difference with the 2nd element. (see my screenshot)

Reg range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f2cc5afecb1f3553d4f400b88bdfe2d3991ef8c0&tochange=3c837b3a38fa15b655c4115cb95eda741b55a819

Comment 2

[Loic]

Mason is on PTO, any thoughts, Markus?

Comment 3

[Sergei Vasiliev]

Attachment: ff45_24112015.png

ff45_24112015.png

Still can reproduce issue with first in ff45.0a1 (2015-11-24).

Comment 4

[Mason Chang [Inactive] [:mchang]]

What's interesting about the first box shadow is that both Chrome / Safari /Edge show an empty box instead of any box shadow, so I think our implementation in Firefox 42 is also wrong.

Comment 5

[Mason Chang [Inactive] [:mchang]]

Attachment: Part 1: Only draw min inset surface if src is equal to dest

The first box shadow in the jsfiddle had a bug where we'd paint the min inset box shadow as a draw surface at [1]. This is because the spread radius was negative, and after extending the destination, the slice would become 0, causing us to stretch the min inset blur to the final destination rect. This patch will only draw the surface if the destination rect is the size of the source rect, which should only happen if the min rects are bigger than the dest rects, since we snap to those sizes at [2].

[1] https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxBlur.cpp?from=gfxBlur.cpp#1050
[2] https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxBlur.cpp?from=gfxBlur.cpp#847

Comment 6

[Mason Chang [Inactive] [:mchang]]

Attachment: Fallback to blur dest rect if a large shadow offset

The second box shadow has a bug because the offset attached to the box-shadow is larger than the min inset blur's size. Before the inset box shadow rewrite, we would fill the area between the inner and outer rect with the shadow color, then blur the edges. The shadow offset would move the inner rect by the offset, then fill the difference.

Now, we create a min rect, blur it, and position the blur to the offset location and draw there. This is normally fine. However, in this case, the offset was large enough to push the min inner rect further than the slice area we'd normally paint onto the destination.  e.g. Imagine a 100x100 div with a blur radius of 1 and an offset of 50px. We'd create a very small min box shadow but the offset would be much bigger than the box shadow size. The area between the actual blur and the div needs to be filled. In these cases, this patch falls back to just blurring the destination rect.

Comment 7

[Mason Chang [Inactive] [:mchang]]

Successful tries:

Part 1 try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22054c1a321f
Part 2 try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=33be8d100f0a

Comment 8

[Eric Rahm [:erahm]]

Clearing MemShrink for now, :BenWa can add it back if it was intentional.

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 8694262 [details] [diff] [review]
Fallback to blur dest rect if a large shadow offset

Review of attachment 8694262 [details] [diff] [review]:
-----------------------------------------------------------------

I suppose it would be nice if our minimum surface didn't have to care about what the outer shape looks like it and where it's positioned in it, but that's for another bug. For example, we could use the minimum surface to only draw the blurred parts of the shadow, and then use solid color FillRects to fill out the rest of the outer shape.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9844966d2fb
https://hg.mozilla.org/integration/mozilla-inbound/rev/46c095a4311b

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b9844966d2fb
https://hg.mozilla.org/mozilla-central/rev/46c095a4311b