Bug 38235 - Remove the unnecessary texImage2D function with Image as input in GraphicsContext3D
Summary: Remove the unnecessary texImage2D function with Image as input in GraphicsCon...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-27 17:27 PDT by Zhenyao Mo
Modified: 2010-04-29 00:42 PDT (History)
6 users (show)

See Also:

Attachments
patch (19.57 KB, patch)
2010-04-27 19:10 PDT, Zhenyao Mo 		no flags Details | Formatted Diff | Diff
revised patch : responding to Ken Russell's review (20.49 KB, patch)
2010-04-28 11:03 PDT, Zhenyao Mo 		no flags Details | Formatted Diff | Diff
revised patch: remove the -image postfix (20.27 KB, patch)
2010-04-28 11:27 PDT, Zhenyao Mo 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Zhenyao Mo 2010-04-27 17:27:22 PDT 
extractImageData should happen in WebGLRenderingContext so we can track format/internalformat/etc. for conformance.
Comment 1 Zhenyao Mo 2010-04-27 19:10:31 PDT 
Created attachment 54503 [details]
patch
Comment 2 Kenneth Russell 2010-04-27 19:21:49 PDT 
Comment on attachment 54503 [details]
patch

Thanks for fixing this. Overall it looks good.

Could you add a helper texImage2D/texSubImage2D taking an Image* and refactor the versions taking HTMLCanvasElement, HTMLImageElement and HTMLVideoElement in terms of it to share more code?

Also, could you name the new routines tex{Sub}Image2DBase or tex{Sub}Image2DImpl? Changing the beginning of the method name is confusing.
Comment 3 Zhenyao Mo 2010-04-28 11:03:09 PDT 
Created attachment 54588 [details]
revised patch : responding to Ken Russell's review
Comment 4 Kenneth Russell 2010-04-28 11:13:30 PDT 
Comment on attachment 54588 [details]
revised patch : responding to Ken Russell's review

Please just use overloaded methods for texImage2DImage and texSubImage2DImage -- i.e., drop the "Image" suffix. The "Base" version is fine. Thanks.
Comment 5 Zhenyao Mo 2010-04-28 11:27:03 PDT 
Created attachment 54593 [details]
revised patch: remove the -image postfix
Comment 6 Kenneth Russell 2010-04-28 11:33:32 PDT 
Comment on attachment 54593 [details]
revised patch: remove the -image postfix

Thanks, this is a nice cleanup. Looks good to me.
Comment 7 Dimitri Glazkov (Google) 2010-04-28 12:56:29 PDT 
Comment on attachment 54593 [details]
revised patch: remove the -image postfix

rs=me.
Comment 8 WebKit Commit Bot 2010-04-29 00:42:21 PDT 
Comment on attachment 54593 [details]
revised patch: remove the -image postfix

Clearing flags on attachment: 54593

Committed r58481: <http://trac.webkit.org/changeset/58481>
Comment 9 WebKit Commit Bot 2010-04-29 00:42:27 PDT 
All reviewed patches have been landed.  Closing bug.