Bug 115360 – [BlackBerry] Replace disappearing fillBuffer() API with graphics context drawing

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Jakob Petsovits]

Instead of using fillBuffer() to draw directly to the target buffer, we now lock a Drawable on it and fill it with PlatformGraphicsContext::addPredefinedPattern().

As a bonus, this also includes related clean-ups - simpler checkerboard painting code, removal of fillWindow(), clearWindow() and paintDefaultBackground(), as well as getting rid of the DEBUG_CHECKERBOARD define which has been useless for performance tracing purposes for a while now.

Internally tracked as PR 303048. Patch below.

[Jakob Petsovits]

Created attachment 200019 [details]
Patch

[Mike Lattanzio]

Comment on attachment 200019 [details]
Patch

Looks great.

[Konrad Piascik]

Comment on attachment 200019 [details]
Patch

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

just those few nits...otherwise looks good

> Source/WebKit/blackberry/Api/BackingStore.cpp:1270
>                  std::vector<Platform::IntRect> dirtyRenderedRects = renderedRegion.rects();

std vs WTF vector

> Source/WebKit/blackberry/Api/BackingStore.cpp:1304
> +            std::vector<Platform::IntRect> overScrollRects = overScrollRegion.rects();

std vs WTF vector?

> Source/WebKit/blackberry/Api/BackingStore.cpp:1338
> +            overlayContext->setStrokeColor(0xff0000ff);

do you want to maybe save the state of the context before modifying it?

[Jakob Petsovits]

Created attachment 200028 [details]
Patch

[Jakob Petsovits]

(In reply to comment #3)
> (From update of attachment 200019 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=200019&action=review
> 
> just those few nits...otherwise looks good
> 
> > Source/WebKit/blackberry/Api/BackingStore.cpp:1270
> >                  std::vector<Platform::IntRect> dirtyRenderedRects = renderedRegion.rects();
> 
> std vs WTF vector

This one actually gets its list of rects from Platform::IntRectRegion, which returns them as std::vector. Given that we drop those again right afterwards, I don't believe it makes sense to convert this to WTF::Vector.

However, in an attempt to appease you, I made an improvement by taking the rects inside the for-loop as const reference and therefore avoiding a copy (unless the compiler took care of that by itself even otherwise).

> > Source/WebKit/blackberry/Api/BackingStore.cpp:1338
> > +            overlayContext->setStrokeColor(0xff0000ff);
> 
> do you want to maybe save the state of the context before modifying it?

Yes, I think I want that. Added in the new version.

[Konrad Piascik]

Comment on attachment 200028 [details]
Patch

Ok.  Looks good, I'd r+ but not a reviewer yet :)

[Rob Buis]

Comment on attachment 200028 [details]
Patch

Ok, good to see so much code removal.

[WebKit Commit Bot]

Comment on attachment 200028 [details]
Patch

Clearing flags on attachment: 200028

Committed r149336: <http://trac.webkit.org/changeset/149336>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.