Bug 137776

Summary:

Allow reuse of BitmapTextureImageBuffer instances

Product:

WebKit

Reporter:

Adrien Destugues <pulkomandy>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

cdumez, cmarcelo, commit-queue, kondapallykalyan, luiz, noam, rniwa, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Adrien Destugues

Reported 2014-10-16 05:57:17 PDT

Allow reuse of BitmapTextureImageBuffer instances

Attachments
Patch (2.19 KB, patch) 2014-10-16 06:02 PDT, Adrien Destugues	no flags	Details Formatted Diff Diff
Patch (2.17 KB, patch) 2014-10-16 13:52 PDT, Adrien Destugues	no flags	Details Formatted Diff Diff
Patch (2.47 KB, patch) 2014-10-16 13:58 PDT, Adrien Destugues	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Adrien Destugues

Comment 1 2014-10-16 06:02:01 PDT

Created attachment 239945 [details] Patch

Chris Dumez

Comment 2 2014-10-16 13:31:32 PDT

Comment on attachment 239945 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239945&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperImageBuffer.h:36 > + bool canReuseWith(const IntSize& /* contentsSize */, Flags = 0) { return true; } Isn't this function virtual? if so, please mark it explicitly as virtual. You will also want to use override (and likely final).

Chris Dumez

Comment 3 2014-10-16 13:41:50 PDT

Comment on attachment 239945 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239945&action=review Someone else should comment on the general approach. >> Source/WebCore/platform/graphics/texmap/TextureMapperImageBuffer.h:36 >> + bool canReuseWith(const IntSize& /* contentsSize */, Flags = 0) { return true; } > > Isn't this function virtual? if so, please mark it explicitly as virtual. You will also want to use override (and likely final). Yes, it is virtual, please use: virtual bool canReuseWith(const IntSize& /* contentsSize */, Flags = 0) override { return true; } Also, please mark the class as final (I don't think anyone subclasses it).

Adrien Destugues

Comment 4 2014-10-16 13:52:33 PDT

Created attachment 239962 [details] Patch

Chris Dumez

Comment 5 2014-10-16 13:54:08 PDT

Comment on attachment 239962 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239962&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperImageBuffer.h:36 > + virtual bool canReuseWith(const IntSize& /* contentsSize */, Flags = 0) override final { return true; } Cannot the class be final instead?

Adrien Destugues

Comment 6 2014-10-16 13:58:57 PDT

Created attachment 239964 [details] Patch

Brent Fulgham

Comment 7 2014-10-31 10:56:29 PDT

Comment on attachment 239964 [details] Patch This seems fine, but the gtk bot seems to fail with this change, and that's one of the builds that would make use of this change.

Michael Catanzaro

Comment 8 2016-09-17 07:04:50 PDT

Comment on attachment 239964 [details] Patch Hi, Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting. To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.

Note You need to log in before you can comment on or make changes to this bug.