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 treegraph
 
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

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>