Bug 101860 – RenderStyle: Pack Color members tighter in substructures.

• Status: REOPENED
• Component: Layout and Rendering

[Andreas Kling]

WebCore::Color is currently quite wasteful with its { RGBA32 rgba; bool isValid; } layout.
For colors that can never be invalid, we should just store an RGBA32. For others, we can pack the validity flags with other bitfield members.

[Andreas Kling]

Created attachment 173497 [details]
EWS experiment

[WebKit Review Bot]

Attachment 173497 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/rendering/style/RenderStyle..." exit_code: 1
Source/WebCore/rendering/style/RenderStyle.cpp:977:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 173497 [details]
EWS experiment

Attachment 173497 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14792714

[Early Warning System Bot]

Comment on attachment 173497 [details]
EWS experiment

Attachment 173497 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14813025

[EFL EWS Bot]

Comment on attachment 173497 [details]
EWS experiment

Attachment 173497 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14809114

[WebKit Review Bot]

Comment on attachment 173497 [details]
EWS experiment

Attachment 173497 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14794644

[Peter Beverloo (cr-android ews)]

Comment on attachment 173497 [details]
EWS experiment

Attachment 173497 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14794646

[Andreas Kling]

Created attachment 173510 [details]
EWS experiment 2

[Andreas Kling]

Created attachment 173515 [details]
Patch

[Antti Koivisto]

Comment on attachment 173515 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173515&action=review

> Source/WebCore/rendering/style/RenderStyle.cpp:978
>  Color RenderStyle::color() const { return inherited->color; }
>  Color RenderStyle::visitedLinkColor() const { return inherited->visitedLinkColor; }
> -void RenderStyle::setColor(const Color& v) { SET_VAR(inherited, color, v) };
> -void RenderStyle::setVisitedLinkColor(const Color& v) { SET_VAR(inherited, visitedLinkColor, v) }
> +void RenderStyle::setColor(const Color& v) { SET_VAR(inherited, color, v.rgb()); }
> +void RenderStyle::setVisitedLinkColor(const Color& v) { SET_VAR(inherited, visitedLinkColor, v.rgb()); }

Should these just take/return RGB32s? It would be good if the interface would make it clear what is being saved. 

At least there should be ASSERT(c.isValid()).

[Andreas Kling]

Committed r134224: <http://trac.webkit.org/changeset/134224>

[WebKit Review Bot]

Re-opened since this is blocked by bug 101937