Bug 235100

Summary: [LBSE] Begin layer-aware RenderSVGContainer implementation
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, rbuis, sabouhallawa, schenney, sergio, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 235099    
Bug Blocks: 90738, 235101    
Attachments:
Description Flags
Patch, v1
none
Patch, v2 rbuis: review+

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>