Bug 235100 - [LBSE] Begin layer-aware RenderSVGContainer implementation
Summary: [LBSE] Begin layer-aware RenderSVGContainer implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords: InRadar
Depends on: 235099
Blocks: 90738 235101
  Show dependency tree / graph
 
Reported: 2022-01-12 02:25 PST by Nikolas Zimmermann
Modified: 2022-01-12 05:06 PST (History)
15 users (show)

See Also:

Attachments
Patch, v1 (33.90 KB, patch)
2022-01-12 02:36 PST, Nikolas Zimmermann 		no flags Details | Formatted Diff | Diff
Patch, v2 (33.99 KB, patch)
2022-01-12 04:11 PST, Nikolas Zimmermann 		rbuis: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2022-01-12 02:25:54 PST 
Now that RenderSVGContainer was renamed to LegacyRenderSVGContainer, we can re-introduce RenderSVGContainer for LBSE.
Comment 1 Nikolas Zimmermann 2022-01-12 02:36:28 PST 
Created attachment 448919 [details]
Patch, v1
Comment 2 Rob Buis 2022-01-12 03:14:42 PST 
Comment on attachment 448919 [details]
Patch, v1

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

> Source/WebCore/rendering/svg/RenderSVGContainer.cpp:6
> + * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>

Likely could use Igalia 2021/2022 copyrights.

> Source/WebCore/rendering/svg/RenderSVGContainer.cpp:126
> +    if (style().display() == DisplayType::None)

Can this actually happen?

> Source/WebCore/rendering/svg/RenderSVGContainer.cpp:178
> +    // Give RenderSVGViewportContainer a chance to apply its viewport clip

Usually we end sentences with a period.

> Source/WebCore/rendering/svg/RenderSVGContainer.h:5
> + * Copyright (C) 2009 Apple Inc. All rights reserved.

Ditto.

> Source/WebCore/rendering/svg/RenderSVGContainer.h:55
> +    bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override;

Please check whether any of the overrides can actually be final.
Comment 3 Nikolas Zimmermann 2022-01-12 04:06:24 PST 
(In reply to Rob Buis from comment #2)
> > Source/WebCore/rendering/svg/RenderSVGContainer.cpp:6
> > + * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
> 
> Likely could use Igalia 2021/2022 copyrights.
Fixed.

> > Source/WebCore/rendering/svg/RenderSVGContainer.cpp:126
> > +    if (style().display() == DisplayType::None)
> 
> Can this actually happen?
Yes -- e.g. <defs> creates a renderer even though "display: none" is set.
 
> > Source/WebCore/rendering/svg/RenderSVGContainer.cpp:178
> > +    // Give RenderSVGViewportContainer a chance to apply its viewport clip
> 
> Usually we end sentences with a period.
Fixed.

> > Source/WebCore/rendering/svg/RenderSVGContainer.h:5
> > + * Copyright (C) 2009 Apple Inc. All rights reserved.
> 
> Ditto.
Fixed.

> 
> > Source/WebCore/rendering/svg/RenderSVGContainer.h:55
> > +    bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override;
> 
> Please check whether any of the overrides can actually be final.
Unfortunately not, they are all overrides in some of the many classes that derive from RenderSVGContainer.
Comment 4 Nikolas Zimmermann 2022-01-12 04:11:27 PST 
Created attachment 448931 [details]
Patch, v2
Comment 5 Nikolas Zimmermann 2022-01-12 05:05:57 PST 
Committed r287921 (245954@trunk): <https://commits.webkit.org/245954@trunk>
Comment 6 Radar WebKit Bug Importer 2022-01-12 05:06:19 PST 
<rdar://problem/87451223>