Bug 110911 - Refactor RenderSVGModelObject on top of RenderLayerModelObject
Summary: Refactor RenderSVGModelObject on top of RenderLayerModelObject
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Florin Malita
URL:
Keywords:
Depends on: 90738
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-26 14:39 PST by Florin Malita
Modified: 2022-07-15 16:07 PDT (History)
14 users (show)

See Also:


Attachments
Patch (3.78 KB, patch)
2013-02-26 14:47 PST, Florin Malita
krit: review+
krit: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florin Malita 2013-02-26 14:39:54 PST
Small step towards https://bugs.webkit.org/show_bug.cgi?id=90738
Comment 1 Florin Malita 2013-02-26 14:47:48 PST
Created attachment 190365 [details]
Patch
Comment 2 Dirk Schulze 2013-02-26 15:19:44 PST
Comment on attachment 190365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190365&action=review

Looks very promising. r=me with some text enhancement requests.

> Source/WebCore/ChangeLog:10
> +        chain but does not enble any layer-related SVG features.

Please add a description why we do not allow the creation of layers. That we need to verify that SVG transforms still work on elements that create layers.

> Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:113
> +    // FIXME: SVG layers not allowed yet.

Can you add a note that we do not allow it, since we do not know the influence on transforms? I think this is the main problem. When we have tests that verify that transforms work on creating a layer, we are fine. But we would need to have at least one property creating a layer for sure. Not sure where we might even benefit from. opacity?
Comment 3 Florin Malita 2013-02-26 15:48:56 PST
(In reply to comment #2)
> But we would need to have at least one property creating a layer for sure. Not sure where we might even benefit from. opacity?

Yes, opacity+fO are prime candidates: https://bugs.webkit.org/show_bug.cgi?id=23113
Comment 4 Eric Seidel (no email) 2013-02-26 15:51:26 PST
Can we chat about this over VC or #webkit sometime?  I'm not sure this is the right direction for SVG or the rendering tree.
Comment 5 Dirk Schulze 2013-02-26 16:08:22 PST
(In reply to comment #4)
> Can we chat about this over VC or #webkit sometime?  I'm not sure this is the right direction for SVG or the rendering tree.

To be honest, this bug report does not give the full strategy. Therefore it might be unclear where it is going.

To shorten it, we need RenderLayers in some situations to share different capabilities. These are 3D transforms, maybe filters, maybe masking and maybe opacity. To shorten it, everything that can use HW accelerated rendering or where it makes a lot of sense to share the code path with HTML. We should avoid creating layers as much as possible IMO. Transforms are heavily used in SVG. I fear that it is a general overhead to create a RenderLayer for each element.

I am happy to chat about this and I want to have everyone on board.
Comment 6 Eric Seidel (no email) 2013-02-26 16:11:14 PST
Sure.  You want GraphicsLayers for all the 3d stuff.  You'll want something to participate in stacking.  But I don't think you want all the bloat and CSS-specific goop that is RenderLayer.
Comment 7 Nikolas Zimmermann 2021-11-16 14:25:55 PST
The RenderLayer size is smaller nowadays, all the CSS related overflow/scroll handling is moved out to RenderLayerScrollableArea, that a RenderLayer can own, similar to RenderLayerFilters (both optional).

There is still potential for optimizations. Anyhow this bug report no longer blocks 90738 - I'm therefore changing the relationship between the tickets (swap depends <-> blocks).
Comment 8 Brent Fulgham 2022-07-15 16:07:22 PDT
This code has been significantly refactored since this patch was proposed. There doesn't seem to be any action we can take here.