test_shell: fix for newer cairos

test_shell: fix for newer cairos

We need to call cairo_surface_mark_dirty to let Cairo know that
we've twiddled the pixels without its knowledge.  Without this,
on current Cairo versions, test_shell's web widget shows up as
a gray box.

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

[Evan Martin, 2010-07-12 22:35:23]

really freaked me out the first time my empty test shell came up, glad we knew
about this cairo bug

[Elliot Glaysher, 2010-07-12 22:39:54]

OK

On Mon, Jul 12, 2010 at 3:35 PM,  <evan@chromium.org> wrote:
> Reviewers: Elliot Glaysher, Evan Stade,
>
> Message:
> really freaked me out the first time my empty test shell came up, glad we
> knew
> about this cairo bug
>
> Description:
> test_shell: fix for newer cairos
>
> We need to call cairo_surface_mark_dirty to let Cairo know that
> we've twiddled the pixels without its knowledge.  Without this,
> on current Cairo versions, test_shell's web widget shows up as
> a gray box.
>
> Please review this at http://codereview.chromium.org/2982002/show
>
> Affected files:
>  M skia/ext/bitmap_platform_device_linux.cc
>
>
> Index: skia/ext/bitmap_platform_device_linux.cc
> diff --git a/skia/ext/bitmap_platform_device_linux.cc
> b/skia/ext/bitmap_platform_device_linux.cc
> index
>
f170273fc38ff0479fbabea2363918946732a02d..9b589de3254ffefd8cd1aa3d97d21fd2aa8a67d9
> 100644
> --- a/skia/ext/bitmap_platform_device_linux.cc
> +++ b/skia/ext/bitmap_platform_device_linux.cc
> @@ -187,7 +187,11 @@ BitmapPlatformDevice::~BitmapPlatformDevice() {
>  }
>
>  cairo_t* BitmapPlatformDevice::beginPlatformPaint() {
> -  return data_->GetContext();
> +  cairo_t* cairo = data_->GetContext();
> +  // Tell Cairo that we've (probably) modified its pixel buffer without
> +  // its knowledge.
> +  cairo_surface_mark_dirty(cairo_get_target(cairo));
> +  return cairo;
>  }
>
>  void BitmapPlatformDevice::setMatrixClip(const SkMatrix& transform,
>
>
>

[Evan Martin, 2010-07-12 22:42:39]

+brettw fyi, as the skia master

[brettw at google, 2010-07-12 22:54:21]

Is the ordering important? Is it OK to call this in
beginPlatformPaint, because we can also put it in endPlatformPaint.

Brett

On Mon, Jul 12, 2010 at 3:39 PM, Elliot Glaysher (Chromium)
<erg@chromium.org> wrote:
> OK
>
> On Mon, Jul 12, 2010 at 3:35 PM,  <evan@chromium.org> wrote:
>> Reviewers: Elliot Glaysher, Evan Stade,
>>
>> Message:
>> really freaked me out the first time my empty test shell came up, glad we
>> knew
>> about this cairo bug
>>
>> Description:
>> test_shell: fix for newer cairos
>>
>> We need to call cairo_surface_mark_dirty to let Cairo know that
>> we've twiddled the pixels without its knowledge.  Without this,
>> on current Cairo versions, test_shell's web widget shows up as
>> a gray box.
>>
>> Please review this at http://codereview.chromium.org/2982002/show
>>
>> Affected files:
>>  M skia/ext/bitmap_platform_device_linux.cc
>>
>>
>> Index: skia/ext/bitmap_platform_device_linux.cc
>> diff --git a/skia/ext/bitmap_platform_device_linux.cc
>> b/skia/ext/bitmap_platform_device_linux.cc
>> index
>>
f170273fc38ff0479fbabea2363918946732a02d..9b589de3254ffefd8cd1aa3d97d21fd2aa8a67d9
>> 100644
>> --- a/skia/ext/bitmap_platform_device_linux.cc
>> +++ b/skia/ext/bitmap_platform_device_linux.cc
>> @@ -187,7 +187,11 @@ BitmapPlatformDevice::~BitmapPlatformDevice() {
>>  }
>>
>>  cairo_t* BitmapPlatformDevice::beginPlatformPaint() {
>> -  return data_->GetContext();
>> +  cairo_t* cairo = data_->GetContext();
>> +  // Tell Cairo that we've (probably) modified its pixel buffer without
>> +  // its knowledge.
>> +  cairo_surface_mark_dirty(cairo_get_target(cairo));
>> +  return cairo;
>>  }
>>
>>  void BitmapPlatformDevice::setMatrixClip(const SkMatrix& transform,
>>
>>
>>
>

[Evan Martin, 2010-07-12 23:06:00]

On 2010/07/12 22:54:21, brettw at google wrote:
> Is the ordering important? Is it OK to call this in
> beginPlatformPaint, because we can also put it in endPlatformPaint.

I believe it is correct.  This function call is saying: "cairo, we have modified
your backing store, so please drop any cached info you have before doing
anything new".  Since the function is retrieving the cairo_t presumably the
caller wants to make cairo calls on it (as opposed to skia calls, which would
continue modifying the backing store).

There's a symmetric call in the other way (cairo_flush, to flush cairo drawing
commands back out to the backing store) that would belong in endPaint, but we
don't have an endPaint and we don't ever draw with cairo then with skia.

[brettw, 2010-07-12 23:48:19]

Got it, LGTM

On Mon, Jul 12, 2010 at 4:06 PM,  <evan@chromium.org> wrote:
> On 2010/07/12 22:54:21, brettw at google wrote:
>>
>> Is the ordering important? Is it OK to call this in
>> beginPlatformPaint, because we can also put it in endPlatformPaint.
>
> I believe it is correct.  This function call is saying: "cairo, we have
> modified
> your backing store, so please drop any cached info you have before doing
> anything new".  Since the function is retrieving the cairo_t presumably the
> caller wants to make cairo calls on it (as opposed to skia calls, which
> would
> continue modifying the backing store).
>
> There's a symmetric call in the other way (cairo_flush, to flush cairo
> drawing
> commands back out to the backing store) that would belong in endPaint, but
> we
> don't have an endPaint and we don't ever draw with cairo then with skia.
>
> http://codereview.chromium.org/2982002/show
>

[Evan Stade, 2010-07-13 00:33:13]

lgtm