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

Rob Buis
Reported 2021-04-13 05:36:29 PDT
Make SVGElement::getBoundingBox retrieve bbox from RenderObject.
Attachments
Patch (3.21 KB, patch)
2021-04-13 05:37 PDT, Rob Buis 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (3.76 KB, patch)
2021-04-13 06:07 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (6.02 KB, patch)
2021-04-13 08:23 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2021-04-13 05:37:57 PDT
Created attachment 425865 [details] Patch
Rob Buis
Comment 2 2021-04-13 06:07:03 PDT
Created attachment 425867 [details] Patch
Rob Buis
Comment 3 2021-04-13 08:23:35 PDT
Created attachment 425873 [details] Patch
alan
Comment 4 2021-04-13 13:25:25 PDT
Wouldn't this return incorrect rect for SVGPathElement?
Rob Buis
Comment 5 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.
alan
Comment 6 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.
alan
Comment 7 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.
Rob Buis
Comment 8 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.
EWS
Comment 9 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].
Radar WebKit Bug Importer
Comment 10 2021-04-14 00:01:17 PDT
<rdar://problem/76631485>
alan
Comment 11 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.
Note You need to log in before you can comment on or make changes to this bug.