nsHTMLCanvasFrame::ComputeSize has duplicated intrinsic size code
Categories
(Core :: Layout, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox68 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
nsHTMLCanvasFrame::ComputeSize right now has this chunk of code:
nsIntSize size = GetCanvasSize();
IntrinsicSize intrinsicSize;
intrinsicSize.width.SetCoordValue(
nsPresContext::CSSPixelsToAppUnits(size.width));
intrinsicSize.height.SetCoordValue(
nsPresContext::CSSPixelsToAppUnits(size.height));
This is equivalent to just calling nsHTMLCanvasFrame::GetIntrinsicSize() (which is currently the one-liner return IntrinsicSizeFromCanvasSize(GetCanvasSize()), and IntrinsicSizeFromCanvasSize looks basically identical to the code quoted above.
Let's just call GetIntrinsicSize to simplify things. (GetIntrinsicSize is a virtual function, but both the caller and callee here are in nsHTMLCanvasFrame's own function-implementations, and nsHTMLCanvasFrame is final, so compilers should be able to devirtualize the call.)
|
Assignee | |
Comment 1•5 years ago
|
||
(Motivation: this will make for fewer special cases in bug 1544121.)
|
|
Assignee | |
Comment 2•5 years ago
|
||
Actually, since we're getting both the intrinsic ratio and size in this function, we should pull out the GetCanvasSize() call so that we only call that part once, and use the Intrinsic[Size,Ratio]FromCanvasSize helpers on the result of that.
(This is slightly more optimal than calling GetIntrinsicSize();GetIntrinsicRatio(); would be, because each of those have a GetCanvasSize() call internally.)
|
|
Assignee | |
Comment 3•5 years ago
|
||
This patch should not affect behavior; the new implementation is
identical to the old one, but with better sharing of code.
Also: I'm removing the code-comment saying that the intrinsic ratio
is unused, because it's not really useful and it's unclear to me that
it's strictly true. There are several cases in the function we pass it
to, nsFrame::ComputeSizeWithIntrinsicDimensions, that use the ratio
and that look reachable as long as we have 'width:auto' in CSS.
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6ed35343aea0 Refactor nsHTMLCanvasFrame::ComputeSize to use existing helper functions and avoid redundant GetCanvasSize() call. r=TYLin
|
||
Comment 5•5 years ago
|
||
| bugherder | ||
Description
•