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

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Victoria <vrk at google.com>  2010-07-28 11:24:39 PST ---
(In reply to comment #2)
> (From update of attachment 62789 [details])
> Few tiny nits... we'll probably need vangelis/darin to take a look.
> 
> 
> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:99
>  +      // This is needed to get text to show up correctly. Without it,
> indentation
Thanks! Fixed.


> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:119
>  +  #error "Need to implement for your platform."
> Do you know if this will this break Chromium Mac since they don't use Skia?
Not sure, but it looks to be the case. Not sure how we handle Mac stuff. 

BTW, what I did to implement this version of VideoLayerChromium.cpp is I copied everything in LayerChromium.cpp then changed the code as necessary to make it work for glMapTexSubImage2D, so problems here are also in LayerChromium. But this brings up a related point, there's nothing really video-specific to what I'm doing here in VideoLayerChromium yet. Do you think I should actually be modifying LayerChromium.cpp with these changes, then have VideoLayerChromium.cpp essentially still just inherit LayerChromium.cpp behavior?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:129
>  +      ASSERT(skiaBitmap);
> I think this will always be true since a reftype (const SkBitmap&) can't be NULL (it's sitting on your stack or in heap memory somewhere).  Therefore taking an address of that will always produce a non-NULL address.  Actually do we even need skiaBitmap?  It looks like you can do all operations in this function using bitmap.

Right, again I was copying what was done in LayerChromium.cpp, so I thought it might be some WebKit stylistic thing to make the code arguably more readable by avoiding reftypes. Let me know what you think is best, and I should probably update LayerChromium.cpp to match (or like I said earlier, maybe I should be modifying LayerChromium.cpp to begin with).

> 
> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:158
>  +      ASSERT(skiaBitmap);
> ditto for here
> 
> WebCore/platform/graphics/chromium/VideoLayerChromium.h:59
>  +  
> remove extra lines
Thanks! Done.

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