Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1498603002:
Handled neutered ImageData in texImage2D and texSubImage2D (Closed)
DescriptionHandled neutered ImageData in texImage2D and texSubImage2D
Ideal we would like to throw an exception in this case. But since the
API for these two methods does not have RaiseException, this CL handles
this case the same way as the case when ImageData is null.
BUG=564846
Committed: https://crrev.com/c5c1f8652c7b99ec36753ed056cd4e480ad53865
Cr-Commit-Position: refs/heads/master@{#363069}
Patch Set 1 #
Messages
Total messages: 16 (5 generated)
xidachen@chromium.org changed reviewers: + junov@chromium.org, kbr@chromium.org
PTAL.
This is premature. See comments I made on the bug.
On 2015/12/02 21:14:19, Justin Novosad wrote: > This is premature. See comments I made on the bug. I double checked the texImage2D(ImageData), it appears that the function validateTexFunc() already checks the dimension of the input data to ensure that its width&height is > 0, so this CL will be an overkill.
On 2015/12/02 21:14:19, Justin Novosad wrote: > This is premature. See comments I made on the bug. I double checked the texImage2D(ImageData), it appears that the function validateTexFunc() already checks the dimension of the input data to ensure that its width&height is > 0, so this CL will be an overkill.
On 2015/12/03 02:07:46, xidachen wrote: > On 2015/12/02 21:14:19, Justin Novosad wrote: > > This is premature. See comments I made on the bug. > > I double checked the texImage2D(ImageData), it appears that the function > validateTexFunc() already checks the dimension of the input data to ensure that > its width&height is > 0, so this CL will be an overkill. Not necessarily overkill. There is an argument for throwing an error rather failing silently. Other APIs throw when trying to use neutered objects.
On 2015/12/03 04:04:23, Justin Novosad wrote: > On 2015/12/03 02:07:46, xidachen wrote: > > On 2015/12/02 21:14:19, Justin Novosad wrote: > > > This is premature. See comments I made on the bug. > > > > I double checked the texImage2D(ImageData), it appears that the function > > validateTexFunc() already checks the dimension of the input data to ensure > that > > its width&height is > 0, so this CL will be an overkill. > > Not necessarily overkill. There is an argument for throwing an error rather > failing silently. Other APIs throw when trying to use neutered objects. Hmm...Could you give me an example? I thought throwing an error requires adding RaiseException in the .idl.
On 2015/12/03 04:39:47, xidachen wrote: > On 2015/12/03 04:04:23, Justin Novosad wrote: > > On 2015/12/03 02:07:46, xidachen wrote: > > > On 2015/12/02 21:14:19, Justin Novosad wrote: > > > > This is premature. See comments I made on the bug. > > > > > > I double checked the texImage2D(ImageData), it appears that the function > > > validateTexFunc() already checks the dimension of the input data to ensure > > that > > > its width&height is > 0, so this CL will be an overkill. > > > > Not necessarily overkill. There is an argument for throwing an error rather > > failing silently. Other APIs throw when trying to use neutered objects. > > Hmm...Could you give me an example? I thought throwing an error requires adding > RaiseException in the .idl. WebGL errors are not the same thing as DOM exceptions. kbr?
Thanks for catching this case. A spec edit defining the behavior the way you've defined it here is in progress at https://github.com/KhronosGroup/WebGL/pull/1349 . LGTM
The CQ bit was checked by xidachen@chromium.org
The CQ bit was unchecked by xidachen@chromium.org
The CQ bit was checked by xidachen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1498603002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1498603002/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Handled neutered ImageData in texImage2D and texSubImage2D Ideal we would like to throw an exception in this case. But since the API for these two methods does not have RaiseException, this CL handles this case the same way as the case when ImageData is null. BUG=564846 ========== to ========== Handled neutered ImageData in texImage2D and texSubImage2D Ideal we would like to throw an exception in this case. But since the API for these two methods does not have RaiseException, this CL handles this case the same way as the case when ImageData is null. BUG=564846 Committed: https://crrev.com/c5c1f8652c7b99ec36753ed056cd4e480ad53865 Cr-Commit-Position: refs/heads/master@{#363069} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/c5c1f8652c7b99ec36753ed056cd4e480ad53865 Cr-Commit-Position: refs/heads/master@{#363069} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
