Bug 224482

Summary: Make SVGElement::getBoundingBox retrieve bbox from RenderObject
Product: WebKit Reporter: Rob Buis <rbuis>
Component: SVGAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, dino, esprehn+autocc, ews-watchlist, fmalita, gyuyoung.kim, kangil.han, pdr, sabouhallawa, schenney, sergio, webkit-bug-importer, zalan, zimmermann
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Rob Buis 2021-04-13 05:36:29 PDT 
Make SVGElement::getBoundingBox retrieve bbox from RenderObject.
Comment 1 Rob Buis 2021-04-13 05:37:57 PDT 
Created attachment 425865 [details]
Patch
Comment 2 Rob Buis 2021-04-13 06:07:03 PDT 
Created attachment 425867 [details]
Patch
Comment 3 Rob Buis 2021-04-13 08:23:35 PDT 
Created attachment 425873 [details]
Patch
Comment 4 zalan 2021-04-13 13:25:25 PDT 
Wouldn't this return incorrect rect for SVGPathElement?
Comment 5 Rob Buis 2021-04-13 13:39:09 PDT 
(In reply to zalan from comment #4)
> Wouldn't this return incorrect rect for SVGPathElement?

I do not think so, an SVGPathElement is a SVGGraphicsElement too. There should be no change in behaviour.
Comment 6 zalan 2021-04-13 15:22:54 PDT 
The difference is 
1. path().boundingRect() (SVGPathElement::getBBox -> RenderSVGPath::path().boundingRect()) vs. 
2. some cached path().boundingRect() value (through RenderSVGPath::objectBoundingBox() which returns m_fillBoundingBox which is populated at updateShapeFromElement by calling calculateObjectBoundingBox (which is indeed path().boundingRect()),
but I think it should be fine.
Comment 7 zalan 2021-04-13 15:40:27 PDT 
ResizeObservation::computeObservedSize (and some of the other call sites) are also "change in functionality" but let's consider them progressions.
Comment 8 Rob Buis 2021-04-13 23:52:14 PDT 
(In reply to zalan from comment #7)
> ResizeObservation::computeObservedSize (and some of the other call sites)
> are also "change in functionality" but let's consider them progressions.

Ah sorry, I missed this and the Path specific code, I thought there was only SVGLocatable::getBBox. Thanks for analysing and given it turned out green, the change should be safe.
Comment 9 EWS 2021-04-14 00:00:05 PDT 
Committed r275935 (236497@main): <https://commits.webkit.org/236497@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425873 [details].
Comment 10 Radar WebKit Bug Importer 2021-04-14 00:01:17 PDT 
<rdar://problem/76631485>
Comment 11 zalan 2021-04-14 11:16:48 PDT 
(In reply to Rob Buis from comment #8)
> (In reply to zalan from comment #7)
> > ResizeObservation::computeObservedSize (and some of the other call sites)
> > are also "change in functionality" but let's consider them progressions.
> 
> Ah sorry, I missed this and the Path specific code, I thought there was only
> SVGLocatable::getBBox. Thanks for analysing and given it turned out green,
> the change should be safe.
Thanks for fixing it.