Also looking at this...

stealing since :emilio is busy with more important stuff atm

Dzmitry has debugged this enough to understand that it is rare. Demoting to P4

I think the issue comes from the way we interpolate clip UV coordinates for the case where the clip space is perspective-transformed. Our current approach involves special code for 2D un-projection of a local point into the clip plane ("get_node_pos" in shaders), and one of the following may be happening:

1. the un-projection code doesn't work with perspective: it assumes the space is just a rotated/scaled plane, which isn't exactly the case here
2. these UV coordinates aren't interpolated with perspective correction

Attaching a simple yaml demonstrating how we mess up box-shadows in perspective so I can easily run this on different machines.

Ok so dug through this a bit and we seem to have figured this out. Basically, we are currently overly relying on "preserve-3d" to force us to render things in local space / intermediates. Notably, the "CS" family of shaders aren't supposed to be run with perspective -- they're supposed to be drawn in local space and then have the perspective applied with brush_image.

The case in question draws a box shadow inside a perspective transform without any preserve_3ds. The pipeline then concludes nothing needs to be composited and we get messed up shadows. The attached test-case demonstrates that just adding a preserve-3d node fixes everything. But we can't rely on users adding these, so we need to add some more robust automatic detection.

kvark has suggested this should be done when we're allocating render tasks.

To clarify on my suggestion, in the render task chain Primitive -> BoxShadow -> Output, when there is perspective transformation involved we are currently doing the transform in Primitive -> BoxShadow step, which is done via a "cs_" shader that doesn't support perspective, while BoxShadow -> Output is done via an image (or blend) brush without transformation. Instead, we should be doing Primitive -> BoxShadow without transformation and then apply the perspective to the image/blend shader in the second step.

handing back to kvark since the fix is too subtle for my understanding of these subsystems.

Just to add extra information that has led to some confusion:

:gw asserted that the cs_clip family of shaders should be handling this case correctly. The shader does seem to be partially perspective aware, just not perspective-interpolation aware, in the BRUSH_FLAG_PERSPECTIVE_INTERPOLATION sense.

So this can be either regarded as a bug that we're letting perspective get all the way to this shader, or that the shader is failing to handle perspective interpolation.

ni?gw just to get their two cents on which is the "right" fix.

lemme look at it on my (magical) machine first before ni? gw

Set up position W of clip rendering,
such that everything is perspective-interpolated in clip space.

Did you mean to only attach the reftest with no code changes?

I actually wanted to attach the code only, but thought it could cause issues if we land the test case separately. It would be great if you could land that reftest disabled, so that I can just flip the switch in my PR.

updated

https://hg.mozilla.org/mozilla-central/rev/b50cc2768579

https://hg.mozilla.org/mozilla-central/rev/24f1d034783e

a follow-up to the actual fix and the reftest

https://hg.mozilla.org/mozilla-central/rev/05f479965d04

Is this something we should uplift?

(In reply to Julien Cristau [:jcristau] from comment #31)

Is this something we should uplift?

Julien,
It could be, but I don't think it manifests a lot on real pages, so there is no urgency to ship it.

Thanks!

Hello!
Reproduced the issue using Firefox 63.0a1 (20180709221247) on Windows 10x64.
The issue is verified fixed with Firefox 69.0b8 (20190723212625) on Windows 10.x64, macOS 10.14 and Ubuntu 18.04, the image is rendered as expected while using WebRender.