1424177 - Some Bugzilla drop-down menu widgets are missing outlines when WebRender is enabled

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Chris Peterson [:cpeterson]]

Attachment: Bugzilla_screenshot.png

STR:
1. Enable WebRender
2. Load a Bugzilla bug
3. Edit the bug
4. Examine the bug's drop-down menu widgets

RESULT:
Some of the drop-down menu widgets (such as the Priority and Severity fields) are missing one or more sides of their outline. If you zoom the page, more or different widgets' outlines disappear.

I can reproduce this problem on Windows but not Mac. See the attached screenshot.

Comment 1

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

I can reproduce this. Looking.

Comment 2

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Those drop-down menus are native widget in windows. We use fallback render for native widget in webrender. And the result of fallback render is missing border. Maybe some clipping or rounding issue. I'll keep investigating.

Comment 3

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Slightly enlarge drawtarget size when fallback.

We round the size of display item's bounds as draw target size for fallback. But for
some item, such as native widget, it might draw larger than the draw target because
it paint area is determined by rounding the bounds of display item (rounding the rect
might get one pixel wider than rounding the size). So, this patch always use the result
of rounding the bounds instead of rounding the size to prevent some content get clipped.

MozReview-Commit-ID: Dam5l91Y93Y

Comment 4

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=34e98515ca83bfc82a18ab55aee1b42f910f576a

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8942555 [details] [diff] [review]
Slightly enlarge drawtarget size when fallback.

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

::: gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ +530,5 @@
> +  // Some display item may draw exceed the paintSize, we need prepare a larger
> +  // draw target to contain the result.
> +  auto scaledBounds = bounds * LayoutDeviceToLayerScale(1);
> +  scaledBounds.Scale(scale.width, scale.height);
> +  LayerIntSize dtSize = RoundedToInt(scaledBounds).Size();

Does it make sense to get rid of paintSize entirely and use this calculation instead? It seems like the only difference is that for paintSize we're doing the width*scale directly, but here we're scaling the whole bounds and then recomputing the size from the scaled rect. It seems like this second approach is always going to be more correct, and we just do this instead of what we're currently doing for paintSize.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Also, can we add a reftest that exercises this code, since we don't seem to have one already?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #5)
> Does it make sense to get rid of paintSize entirely and use this calculation
> instead? It seems like the only difference is that for paintSize we're doing
> the width*scale directly, but here we're scaling the whole bounds and then
> recomputing the size from the scaled rect. It seems like this second
> approach is always going to be more correct, and we just do this instead of
> what we're currently doing for paintSize.

To save time I did a try push with this suggestion, but it looks like it causes a bunch of failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c131982f5c9f67a1ff850ee69f17f06653c23f9. So never mind that, I guess.

Comment 8

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Slightly enlarge drawtarget size when fallback. (carry r+: kats)

MozReview-Commit-ID: Dam5l91Y93Y

Comment 9

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add reftest.

MozReview-Commit-ID: GfMRLGCia4Y

Comment 10

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

try with reftest:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa5b4e7cb3cd8004cf8c094b010a2434dd4300cc

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8942778 [details] [diff] [review]
Add reftest.

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

Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4c4ce0c895b
Slightly enlarge drawtarget size when fallback. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0a2cfd2fc7a
Add reftest. r=kats

Comment 13

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/c4c4ce0c895b
https://hg.mozilla.org/mozilla-central/rev/e0a2cfd2fc7a