Bug 211993 – Fix operator== and hash() for ExtendedColor

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Fix operator== and hash() for ExtendedColor

[Simon Fraser (smfr)]

Created attachment 399584 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 399593 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 399595 [details]
Patch

[Sam Weinig]

Comment on attachment 399595 [details]
Patch

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

> Source/WebCore/platform/graphics/Color.h:320
> +// One or both may be extended.

s/may/must/

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/261794/webkit

[Radar WebKit Bug Importer]

<rdar://problem/63319983>

[Darin Adler]

Comment on attachment 399595 [details]
Patch

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

> Source/WebCore/platform/graphics/ExtendedColor.h:78
> +    return a.colorSpace() == b.colorSpace() && areEssentiallyEqual(a.channels(), b.channels());

How did you settle on "essentially equal" as the definition of ==, rather than something closer to floating point "=="?

[Simon Fraser (smfr)]

It seemed like the right thing to do? I didn't have any strong evidence to support either choice.

[Darin Adler]

I would suggest that "==" should be built on top of "==", with no shift to "essentially" done beneath the covers.

In fact, reflecting on this I know it’s a problem for hashing! It‘s critical for hash functions that if two values compute as "==", then they have the same hash. That seems to not be true.

[Simon Fraser (smfr)]

Done in https://trac.webkit.org/changeset/261835/webkit