Add a switch to invert web contents (non-accel only).

Add a switch to invert web contents (non-accel only).

The switch will just be used during development of this feature.
The idea is for this to be turned automatically for Windows users
who enable an inverted (white-on-black) color scheme - see bug
for detail.

This first patch only handles the non-accelerated graphics path;
a subsequent change will handle the accelerated path.

BUG=112944
TEST=manually test with --invert-web-contents switch


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=120928

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=121541

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=121715

[dmazzoni, 2012-02-07 16:14:28]

vandebo: skia.gyp and the new skia code in render_widget.cc
avi: new ipc, new switch

[Avi (use Gerrit), 2012-02-07 16:29:04]

IPC, switch LGTM

I had an initial question as to why you're doing it this way, rather than
filtering in the RWHV as you blit the bitmap, but I suppose this is more
efficient.

/me is glad OS X does this in the display compositor for all apps.

[dmazzoni, 2012-02-07 16:36:18]

On 2012/02/07 16:29:04, Avi wrote:
> I had an initial question as to why you're doing it this way, rather than
> filtering in the RWHV as you blit the bitmap, but I suppose this is more
> efficient.

That was my first thought, and I tried that - but StretchBlt doesn't seem to
support every permutation of format and transfer mode, or else some permutations
must be buggy. Plus, Skia gives us more flexibility here - not only can we
gamma-correct, but we can play with increasing the contrast while inverting, as
OS X does.

Thanks!

[Avi (use Gerrit), 2012-02-07 16:39:07]

Hm. I didn't realize you were using GDI to do the blitting, but thought you were
using Skia there too.

When we move Chrome Win to Aura, we should be able to get inversion from the
compositor for free :)

[dmazzoni, 2012-02-07 16:51:42]

On 2012/02/07 16:39:07, Avi wrote:
> When we move Chrome Win to Aura, we should be able to get inversion from the
> compositor for free :)

Awesome! I'm already working on implementing that code path anyway, to support
full-screen inversion on Chrome OS.

[Avi (use Gerrit), 2012-02-07 17:25:30]

Eeek, I meant that we should be able to get that feature in for nearly free in
runtime cost, not that it's implemented yet... :\

[vandebo (ex-Chrome), 2012-02-07 17:57:11]

Would it make more sense to draw as usual and then invert the result just before
it goes to the screen?  With this approach, we have to spend time inverting
everything that's drawn, not just the result...

https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render...
content/renderer/render_widget.cc:576: canvas->saveLayer(&bounds,
invert_paint_.get());
It's a shame that we have to do this in a layer.  setDrawFilter looked
promising, but it seems to be still in progress or being removed.

https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render...
content/renderer/render_widget.cc:1381: if (!invert_paint_.get()) {
nit: && invert

https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render...
content/renderer/render_widget.cc:1384: // where gamma is typically in the range
1.8...2.2. By approximating
I guess there isn't a way to look up the active gamma?

https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render...
content/renderer/render_widget.cc:1403: OnMsgRepaint(size_);
nit: should we only repaint if invert changed?

[dmazzoni, 2012-02-07 21:00:11]

On Tue, Feb 7, 2012 at 9:57 AM, <vandebo@chromium.org> wrote:

> Would it make more sense to draw as usual and then invert the result just
> before
> it goes to the screen?  With this approach, we have to spend time inverting
> everything that's drawn, not just the result...
>

I'm pretty sure this only inverts once per dirty region. If you refresh the
entire screen, it only gets called once on the final composited result. If
you scroll the page and it needs to redraw two rectangular regions, WebKit
redraws each region from scratch and each one is inverted once before being
sent to the browser process.

Or, did you mean in the backing store? I tried doing it in
content/browser/renderer_host/backing_store_win.cc - it was a nightmare -
StretchDIBits doesn't support all modes (like INVERT) in all permutations,
so it sometimes works and other times flips the picture upside down or has
random errors. Also, I can't manually invert the pixels in the source
bitmap because it's in shared memory, and I can't (easily) modify the
backing store itself, because it's a device-dependent bitmap.

Finally, css3 filters are just starting to land - I may eventually switch
to doing it all there. Consider this implementation just the first version,
to get the ball rolling. It should be reasonable but it doesn't have to be
perfect.

https://chromiumcodereview.**appspot.com/9353012/diff/1/**
>
content/renderer/render_**widget.cc#newcode1381<https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render_widget.cc#newcode1381>
> content/renderer/render_**widget.cc:1381: if (!invert_paint_.get()) {
> nit: && invert
>

Fixed.


> https://chromiumcodereview.**appspot.com/9353012/diff/1/**
>
content/renderer/render_**widget.cc#newcode1384<https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render_widget.cc#newcode1384>
> content/renderer/render_**widget.cc:1384: // where gamma is typically in
>
> the range 1.8...2.2. By approximating
> I guess there isn't a way to look up the active gamma?
>

I'd definitely like to consider this for a future optimization - but I
doubt anyone other than a graphic designer could tell the difference
between 2.2 and 2.0, whereas the difference between 1.0 and 2.0 is pretty
significant.


> https://chromiumcodereview.**appspot.com/9353012/diff/1/**
>
content/renderer/render_**widget.cc#newcode1403<https://chromiumcodereview.appspot.com/9353012/diff/1/content/renderer/render_widget.cc#newcode1403>
> content/renderer/render_**widget.cc:1403: OnMsgRepaint(size_);
> nit: should we only repaint if invert changed?
>

Good idea, I just added a short-circuit at the top of the method.

[vandebo (ex-Chrome), 2012-02-07 21:06:09]

LGTM

[commit-bot: I haz the power, 2012-02-08 00:28:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/9353012/6001

[commit-bot: I haz the power, 2012-02-08 02:51:44]

Change committed as 120928

[commit-bot: I haz the power, 2012-02-10 19:50:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/9353012/13001

[commit-bot: I haz the power, 2012-02-10 21:35:28]

Change committed as 121541