Closed Bug 1424177 Opened 2 years ago Closed 2 years ago

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

Categories

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

57 Branch
Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: cpeterson, Assigned: mtseng)

References

Details

(Whiteboard: [wr-mvp] [triage])

Attachments

(3 files, 1 obsolete file)

Attached image 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.
Whiteboard: [wr-mvp] [triage]
I can reproduce this. Looking.
Assignee: nobody → mtseng
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.
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
Attachment #8942555 - Flags: review?(bugmail)
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.
Also, can we add a reftest that exercises this code, since we don't seem to have one already?
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
Attachment #8942555 - Attachment is obsolete: true
Attached patch Add reftest.Splinter Review
MozReview-Commit-ID: GfMRLGCia4Y
Attachment #8942778 - Flags: review?(bugmail)
Comment on attachment 8942778 [details] [diff] [review]
Add reftest.

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

Thanks!
Attachment #8942778 - Flags: review?(bugmail) → review+
https://hg.mozilla.org/mozilla-central/rev/c4c4ce0c895b
https://hg.mozilla.org/mozilla-central/rev/e0a2cfd2fc7a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
See Also: → 1444185
You need to log in before you can comment on or make changes to this bug.