Closed Bug 1582528 Opened 5 years ago Closed 5 years ago

SVG's clip doesn't seem to restrict the visual/building rect.

Categories

(Core :: Web Painting, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: jrmuizel, Assigned: tnikkel)

References

Details

Attachments

(1 file)

Scroll frames reduce the visual/building rect:
https://searchfox.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#3405

SVG doesn't use a scroll frame to implement it's overflow behaviour.
https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/layout/svg/nsSVGOuterSVGFrame.cpp#770

It doesn't seem to restrict the visual/building rect, but perhaps should?

Flags: needinfo?(matt.woodrow)

Yes, I think it should clip the visible/dirty rect (and Markus agrees).

Flags: needinfo?(matt.woodrow)

I've audited the other users of DisplayListClipState::AutoSaveRestore and only found one other location which re-enters BuildDisplayList and does not modify the building rect: https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/layout/forms/nsHTMLButtonControlFrame.cpp#102
All the other users only build a single display item while the DisplayListClipState::AutoSaveRestore is on the stack, or they already use nsDisplayListBuilder::AutoBuildingDisplayList to modify the building rect.

I wrote the obvious patch without thinking about it at all and pushed that to try to see and it's green on reftests.

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

Looks good! Instead of adding another if, I'd move the intersecting into the existing if that sets the clip.

Perhaps you could verify that this fixes your issue?

Flags: needinfo?(jmuizelaar)
Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/840f7be6f7e3
Restrict visual and dirty rect when building the display list for svg contents that cannot overflow. r=mstange
Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

This fixes the problem that I was seeing. Thanks everyone.

Flags: needinfo?(jmuizelaar)
Blocks: 1582810
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: