Bug 234886 - Add some functions to Color to make debugging using color indicators easier
Summary: Add some functions to Color to make debugging using color indicators easier
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-05 09:52 PST by Sam Weinig
Modified: 2022-01-12 21:44 PST (History)
3 users (show)

See Also:


Attachments
WIP (2.20 KB, patch)
2022-01-09 11:08 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (2.21 KB, patch)
2022-01-10 08:25 PST, Sam Weinig
darin: review+
Details | Formatted Diff | Diff
Patch (2.32 KB, patch)
2022-01-12 10:06 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2022-01-05 09:52:26 PST
Antti wants some helpers on Color to make debugging he does where he uses colors in CSS to indicate things easier. And Antti gets what wants.
Comment 1 Sam Weinig 2022-01-09 11:08:44 PST Comment hidden (obsolete)
Comment 2 Sam Weinig 2022-01-10 08:25:12 PST
Created attachment 448753 [details]
Patch
Comment 3 Darin Adler 2022-01-11 09:20:34 PST
Comment on attachment 448753 [details]
Patch

Do we need to add a keyword to make sure this never gets dead-code stripped?

Should these be non-member functions? In WTFString.cpp, string and asciiDebug are, but String::show is a member function. One advantage is that the functions don’t have to be in the header. But maybe they are easier to call?
Comment 4 Antti Koivisto 2022-01-11 10:23:34 PST
Comment on attachment 448753 [details]
Patch

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

> Source/WebCore/platform/graphics/Color.h:185
> +    ResolvedColorType<SRGBA<uint8_t>> debugComponents() const;

debugRGBA?

How does the output look in the debugger? (wondering if having a simple struct just for this would be somehow better)
Comment 5 Antti Koivisto 2022-01-11 10:26:42 PST
> How does the output look in the debugger? (wondering if having a simple
> struct just for this would be somehow better)

These structs are very opaque so a simple 'struct DebugRGBA' might be nice in any case.
Comment 6 Sam Weinig 2022-01-12 06:54:00 PST
(In reply to Antti Koivisto from comment #5)
> > How does the output look in the debugger? (wondering if having a simple
> > struct just for this would be somehow better)
> 
> These structs are very opaque so a simple 'struct DebugRGBA' might be nice
> in any case.

You're right. I was hoping because the actual members were really simple it would be nicer, but it looks like this:

(lldb) p other.debugComponents()
(WebCore::ResolvedColorType<WebCore::BoundedGammaEncoded<unsigned char, WebCore::SRGBADescriptor> >) $1 = {
  WebCore::ExposedColorType<WebCore::BoundedGammaEncoded<unsigned char, WebCore::SRGBADescriptor>, WebCore::RGBModel<unsigned char, WebCore::RGBBoundedness::Bounded> > = {
    WebCore::BoundedGammaEncoded<unsigned char, WebCore::SRGBADescriptor> = {
      WebCore::RGBAType<unsigned char, WebCore::SRGBADescriptor, WebCore::BoundedGammaEncoded<unsigned char, WebCore::SRGBADescriptor>, WebCore::RGBModel<unsigned char, WebCore::RGBBoundedness::Bounded>, WebCore::SRGBTransferFunction<> > = (red = '\0', green = '\0', blue = '\0', alpha = '\0')
    }
  }
}

I'll try something simpler. May want to use something larger than uint8_t for the components as well so they don't get confused for characters.
Comment 7 Radar WebKit Bug Importer 2022-01-12 09:53:17 PST
<rdar://problem/87465278>
Comment 8 Sam Weinig 2022-01-12 10:06:01 PST
Created attachment 448961 [details]
Patch
Comment 9 EWS 2022-01-12 10:57:58 PST
Committed r287936 (245967@main): <https://commits.webkit.org/245967@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448961 [details].
Comment 10 Sam Weinig 2022-01-12 14:02:50 PST
Output is now:

(lldb) p other.debugRGBA()
(WebCore::Color::DebugRGBA) $0 = (red = 0, green = 0, blue = 0, alpha = 0)
Comment 11 Antti Koivisto 2022-01-12 21:44:55 PST
πŸ‘