[webkit-reviews] review denied: [Bug 54330] [chromium] Cannot use multiple accelerated 2D canvas' in the same render process : [Attachment 86811] comment two parameters to setTextureCacheLimits

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Mike Reed <reed at google.com>'s
request for review:
Bug 54330: [chromium] Cannot use multiple accelerated 2D canvas' in the same
render process
https://bugs.webkit.org/show_bug.cgi?id=54330

Attachment 86811: comment two parameters to setTextureCacheLimits
https://bugs.webkit.org/attachment.cgi?id=86811&action=review

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

DrawingBuffer holds a RefPtr to its associated GraphicsContext3D, so the
GraphicsContext3D will never be destroyed before the DrawingBuffer.

Does this code assuming that the GrContext is the only user of the
SharedGraphicsContext3D?

> WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:53
> +// limit the number of textures we hold in the bitmap->texture cache
> +#define GR_MAX_TEXTURE_COUNT    512
> +// limit the bytes allocated toward textures in the bitmap->texture cache

Comments in WebKit should start with a capital letter and end with a period.

Also, we typically use static variables of the appropriate type (size_t here)
instead of #defines for stuff like this.

> WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:474
> +	   m_grContext->setTextureCacheLimits(GR_MAX_TEXTURE_COUNT,
> +					      GR_MAX_TEXTURE_BYTES);

unnecessary line wrap