[Webkit-unassigned] [Bug 43101] Using glMapTexSubImage2d in VideoLayerChromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43101


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62849|review?                     |review-
               Flag|                            |




--- Comment #11 from David Levin <levin at chromium.org>  2010-08-03 10:56:05 PST ---
(From update of attachment 62849)
Just a few very trivial details that it would be nice to clean up.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +2010-07-27  Victoria Kirst  <vrk at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Added logic to use glMapTexSubImage2D to write video layer to GPU
> +        texture. Also fixes CPU usage problem from previous patch.
> +        https://bugs.webkit.org/show_bug.cgi?id=43101

Please explain why there are no tests added.

"No change in user visible functionality (since it isn't turned on), so no new tests."

When this does become enabled either there needs to be tests or an indication of what tests cover the functionality.


> diff --git a/WebCore/platform/graphics/chromium/VideoLayerChromium.cpp b/WebCore/platform/graphics/chromium/VideoLayerChromium.cpp
>  
> +#include "LayerRendererChromium.h"
> +#if PLATFORM(SKIA)
> +#include "NativeImageSkia.h"
> +#include "PlatformContextSkia.h"
> +#endif

#if'ed includes should come after all other includes.

> +#include "RenderLayerBacking.h"
> +#include "skia/ext/platform_canvas.h"
> +
> +#include <GLES2/gl2.h>
> +
> +#define GL_GLEXT_PROTOTYPES
> +#include <GLES2/gl2ext.h>
> +

> +    // If the texture needs to be reallocated then we must redraw the entire

Please add a "," before then.


> +        // FIXME: Does this take us down a very slow text rendering path?
> +        // FIXME: why is this is a windows-only call ?

*W*hy (and the space before the ? is odd.)


> +    // If the texture id or size changed since last time then we need to tell GL
Please add a "," before then.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.