[Windows] Optionally invert colors when drawing to a WebView's backing store.
Created attachment 124230 [details] Patch
Comment on attachment 124230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124230&action=review > Source/WebCore/css/CSSPrimitiveValueMappings.h:291 > + default: > + ASSERT_NOT_REACHED(); > + break; By adding default here, we won’t get warnings if we accidentally leave out one value from the switch statement. We want those warnings at compile time. To get the run-time assertion we are losing the compile-time check. If we want both, then we need to change the break statements into return statements, and then we can put an ASSERT_NOT_REACHED after the switch statement. > Source/WebKit/win/WebView.cpp:1159 > + gc.fillRect(dirtyRect, Color(0xFF, 0xFF, 0xFF), ColorSpaceDeviceRGB, CompositeDifference); Should we use Color::white or 0xFFFFFFFF instead of Color(0xFF, 0xFF, 0xFF)? > Source/WebKit/win/WebView.h:1004 > + bool m_shouldInvertColors; A public data member is not the right style for something you can change on a WebView. Normally we’d use a getter/setter function pair or something along those lines.
(In reply to comment #2) > (From update of attachment 124230 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124230&action=review > > > Source/WebCore/css/CSSPrimitiveValueMappings.h:291 > > + default: > > + ASSERT_NOT_REACHED(); > > + break; > > By adding default here, we won’t get warnings if we accidentally leave out one value from the switch statement. We want those warnings at compile time. To get the run-time assertion we are losing the compile-time check. > > If we want both, then we need to change the break statements into return statements, and then we can put an ASSERT_NOT_REACHED after the switch statement. This is in a ctor so I think I can just remove the default label to achieve what we want here. > > > Source/WebKit/win/WebView.cpp:1159 > > + gc.fillRect(dirtyRect, Color(0xFF, 0xFF, 0xFF), ColorSpaceDeviceRGB, CompositeDifference); > > Should we use Color::white or 0xFFFFFFFF instead of Color(0xFF, 0xFF, 0xFF)? Sure. I'll use Color::white > > > Source/WebKit/win/WebView.h:1004 > > + bool m_shouldInvertColors; > > A public data member is not the right style for something you can change on a WebView. Normally we’d use a getter/setter function pair or something along those lines. This is in the private block, isn't it?
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 124230 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=124230&action=review > > > > > Source/WebKit/win/WebView.h:1004 > > > + bool m_shouldInvertColors; > > > > A public data member is not the right style for something you can change on a WebView. Normally we’d use a getter/setter function pair or something along those lines. > > This is in the private block, isn't it? I should add that I plan to add the API to enable this feature in a follow-on patch, hence the private variable with no getter or setter.
Created attachment 124246 [details] Patch
Comment on attachment 124246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124246&action=review You should probably file bugs on other graphics backends to implement difference. Especially Cairo, since they use win/WebView. > Source/WebKit/win/WebView.cpp:335 > + : m_shouldInvertColors(false) > + , m_refCount(0) I would be good if refCount remained first.
Committed r106274: <http://trac.webkit.org/changeset/106274>