[webkit-reviews] review granted: [Bug 31562] Implement all cases of texImage2d and texSubImage2d : [Attachment 44995] Patch with test case

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 31562: Implement all cases of texImage2d and texSubImage2d
https://bugs.webkit.org/show_bug.cgi?id=31562

Attachment 44995: Patch with test case
https://bugs.webkit.org/attachment.cgi?id=44995&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: WebCore/ChangeLog
> ===================================================================

> +	   This implements all cases except HTMLVideoElement. That will come in

> +	   a subsequent patch. It also does not implement the flipY or
premultiplyAlpha 
> +	   flags which is covered in
https://bugs.webkit.org/show_bug.cgi?id=32620. 
> +	   The code also does not check for buffer overruns. That is covered in

> +	   https://bugs.webkit.org/show_bug.cgi?id=32619.

I don't think your changelog needs to explain what isn't done yet, but it
should say more about
what this particular set of changes is doing.



>  JSValue JSWebGLRenderingContext::texImage2D(ExecState* exec, const ArgList&
args)
>  { 
> -    if (args.size() < 3)
> +    if (args.size() < 3 || args.size() > 9)
>	   return throwError(exec, SyntaxError);
> -
> +    
>      ExceptionCode ec = 0;
> +    
>      WebGLRenderingContext* context =
static_cast<WebGLRenderingContext*>(impl());	
>      unsigned target = args.at(0).toInt32(exec);
>      if (exec->hadException())    
>	   return jsUndefined();
> -	   
> +    
>      unsigned level = args.at(1).toInt32(exec);
>      if (exec->hadException())    
>	   return jsUndefined();
>      
...
> -	       
> +	   
> +	   // This must be the WebGLArray case
>	   unsigned internalformat = args.at(2).toInt32(exec);
>	   if (exec->hadException())	
>	       return jsUndefined();
> -
> +	   
>	   unsigned width = args.at(3).toInt32(exec);
>	   if (exec->hadException())	
>	       return jsUndefined();
> -
> +	   
>	   unsigned height = args.at(4).toInt32(exec);
>	   if (exec->hadException())	
>	       return jsUndefined();
> -
> +	   
>	   unsigned border = args.at(5).toInt32(exec);
>	   if (exec->hadException())	
>	       return jsUndefined();
> -
> +	   
>	   unsigned format = args.at(6).toInt32(exec);
>	   if (exec->hadException())	
>	       return jsUndefined();
> -
> +	   
>	   unsigned type = args.at(7).toInt32(exec);
>	   if (exec->hadException())	
>	       return jsUndefined();

Lots of whitespace changes here.