Summary: | Apply transferred min/max sizes for intrinsic sizing | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||||||||
Component: | CSS | Assignee: | Rob Buis <rbuis> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, changseok, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 47738 | ||||||||||||||||
Attachments: |
|
Description
Rob Buis
2021-03-01 09:14:30 PST
Created attachment 421833 [details]
Patch
Created attachment 422573 [details]
Patch
Created attachment 422601 [details]
Patch
Comment on attachment 422601 [details]
Patch
Not sure about the percentage height code, let me know if there is a better way.
Comment on attachment 422601 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422601&action=review > Source/WebCore/rendering/RenderBlock.cpp:263 > + bool m_hasAspectRatioPercentageHeightDescendant; This probably introduced several bytes of padding. Is there a spare bit you can fit this into? These "descendant has" bits are notoriously hard to keep up-to-date accurately. Is there a way to fix this without storing state like this? > Source/WebCore/rendering/RenderBlock.cpp:2437 > + const_cast<RenderBlock*>(this)->setHasAspectRatioPercentageHeightDescendant(); Yuck. Created attachment 422679 [details]
Patch
Comment on attachment 422601 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422601&action=review >> Source/WebCore/rendering/RenderBlock.cpp:2437 >> + const_cast<RenderBlock*>(this)->setHasAspectRatioPercentageHeightDescendant(); > > Yuck. Agreed, but sometimes it can't be avoided :) Anyway I hope I have a better solution now. Comment on attachment 422679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422679&action=review > Source/WebCore/rendering/RenderBlock.cpp:842 > + if (renderer->hasAspectRatio() || renderer->style().hasAspectRatio()) This is confusing; I feel like renderer->hasAspectRatio() could be renamed (maybe hasIntrinsicAspectRatio()). We have other properties with the same name on both style and renderer; they generally mean the same thing, but in situations where a property does not apply to some kinds of renderers. This one is different though. Created attachment 422868 [details]
Patch
Comment on attachment 422679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422679&action=review >> Source/WebCore/rendering/RenderBlock.cpp:842 >> + if (renderer->hasAspectRatio() || renderer->style().hasAspectRatio()) > > This is confusing; I feel like renderer->hasAspectRatio() could be renamed (maybe hasIntrinsicAspectRatio()). > > We have other properties with the same name on both style and renderer; they generally mean the same thing, but in situations where a property does not apply to some kinds of renderers. This one is different though. Agreed, I mixed up child.style().hasAspectRatio() and child.hasAspectRatio() quite some times in development (and vice versa). Fixed now. Created attachment 422906 [details]
Patch
Committed r274287: <https://commits.webkit.org/r274287> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422906 [details]. |