[webkit-reviews] review denied: [Bug 70645] [chromium] Recycle tile-sized textures during commit to prevent reallocations : [Attachment 117287] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Grace Kloba
<klobag at chromium.org>'s request for review:
Bug 70645: [chromium] Recycle tile-sized textures during commit to prevent
reallocations
https://bugs.webkit.org/show_bug.cgi?id=70645

Attachment 117287: Patch
https://bugs.webkit.org/attachment.cgi?id=117287&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=117287&action=review


This looks very close but there's still some unnecessary complexity here it
looks like.

> Source/WebCore/platform/graphics/chromium/TextureManager.h:52
> +    static PassOwnPtr<TextureManager> create(size_t memoryLimitBytes, size_t
reclaimLimitBytes, int maxTextureSize)

i'm not sure i understand why we need an additional parameter here - we seem to
always set it to the default

> Source/WebCore/platform/graphics/chromium/TextureManager.h:67
> +    void setReclaimLimitBytes(size_t);

I don't understand why this API is being added - in this patch we always
initialize the limit to TextureManager::reclaimLimitBytes() and never set it to
anything else, so why does this need to be configurable at runtime?

> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:263
> +	   tiledLayer->syncTextureId(i, j, tile->texture()->textureId(),
tile->texture()->contentsValid());

what's the advantage of pushing a texture ID over that's not valid? should we
just not sync textures that do not have valid contents to the impl side?