Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(45)

Issue 2866663002: Fix overflow issue when generateMipmap for srgb texture (Closed)

Created:
3 years, 7 months ago by yizhou.jiang
Modified:
3 years, 7 months ago
CC:
chromium-reviews, piman+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Follow up immutable tex srgb emulation issue of generateMipmap. when calculating max available mipmap level, as base level can be set by client to any value, so there is an overflow danger. In addition, when max level of srgb texture is less than full mipmap level, the max level isn't generated. BUG=712096 TEST=conformance2/textures/misc/tex-srgb-mipmap.html CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2866663002 Cr-Commit-Position: refs/heads/master@{#471676} Committed: https://chromium.googlesource.com/chromium/src/+/2e20e662edabca0927a21000850778eef038a7cf

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address to yunchao's feedback #

Total comments: 2

Patch Set 3 : Address to Zhenyao's feedback #

Total comments: 2

Patch Set 4 : Address to Zhenyao's feedback #

Total comments: 1

Patch Set 5 : Address to Zhenyao's feedback #

Patch Set 6 : Modify expectation file #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -10 lines) Patch
M content/test/gpu/gpu_tests/webgl2_conformance_expectations.py View 1 2 3 4 5 2 chunks +2 lines, -1 line 0 comments Download
M gpu/command_buffer/service/gles2_cmd_srgb_converter.cc View 1 2 3 4 2 chunks +15 lines, -9 lines 0 comments Download

Messages

Total messages: 79 (66 generated)
yizhou.jiang
On 2017/05/05 07:47:52, yizhou.jiang wrote: > mailto:yizhou.jiang@intel.com changed reviewers: > + mailto:jiajia.qin@intel.com, mailto:yunchao.he@intel.com Yunchao and ...
3 years, 7 months ago (2017-05-05 07:48:24 UTC) #8
yunchao
Some comments. Please add Jie, who will responsible WebGL 2.0.1 bug fixes. https://codereview.chromium.org/2866663002/diff/1/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc File gpu/command_buffer/service/gles2_cmd_srgb_converter.cc ...
3 years, 7 months ago (2017-05-05 08:01:48 UTC) #9
yizhou.jiang
Thanks for your review yunchao and jiajia, please take another look. The CTS patch is ...
3 years, 7 months ago (2017-05-08 08:28:56 UTC) #15
yunchao
On 2017/05/08 08:28:56, yizhou.jiang wrote: > Thanks for your review yunchao and jiajia, please take ...
3 years, 7 months ago (2017-05-08 08:49:32 UTC) #16
yizhou.jiang
On 2017/05/08 08:49:32, yunchao wrote: > On 2017/05/08 08:28:56, yizhou.jiang wrote: > > Thanks for ...
3 years, 7 months ago (2017-05-08 08:54:33 UTC) #19
Zhenyao Mo
https://codereview.chromium.org/2866663002/diff/20001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc File gpu/command_buffer/service/gles2_cmd_srgb_converter.cc (right): https://codereview.chromium.org/2866663002/diff/20001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc#newcode427 gpu/command_buffer/service/gles2_cmd_srgb_converter.cc:427: (base_level + mipmap_levels - 1) > max_level As I ...
3 years, 7 months ago (2017-05-08 17:25:29 UTC) #20
yizhou.jiang
@Zhenyao Please take another look. Thank you. https://codereview.chromium.org/2866663002/diff/20001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc File gpu/command_buffer/service/gles2_cmd_srgb_converter.cc (right): https://codereview.chromium.org/2866663002/diff/20001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc#newcode427 gpu/command_buffer/service/gles2_cmd_srgb_converter.cc:427: (base_level + ...
3 years, 7 months ago (2017-05-09 15:13:57 UTC) #38
Zhenyao Mo
https://codereview.chromium.org/2866663002/diff/80001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc File gpu/command_buffer/service/gles2_cmd_srgb_converter.cc (right): https://codereview.chromium.org/2866663002/diff/80001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc#newcode481 gpu/command_buffer/service/gles2_cmd_srgb_converter.cc:481: for (GLint level = base_level + 1; level <= ...
3 years, 7 months ago (2017-05-09 17:14:33 UTC) #39
yizhou.jiang
@Zhenyao, please take another look.Thanks. https://codereview.chromium.org/2866663002/diff/80001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc File gpu/command_buffer/service/gles2_cmd_srgb_converter.cc (right): https://codereview.chromium.org/2866663002/diff/80001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc#newcode481 gpu/command_buffer/service/gles2_cmd_srgb_converter.cc:481: for (GLint level = ...
3 years, 7 months ago (2017-05-10 08:28:49 UTC) #51
Zhenyao Mo
https://codereview.chromium.org/2866663002/diff/100001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc File gpu/command_buffer/service/gles2_cmd_srgb_converter.cc (right): https://codereview.chromium.org/2866663002/diff/100001/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc#newcode481 gpu/command_buffer/service/gles2_cmd_srgb_converter.cc:481: for (base::CheckedNumeric<GLint> level = base_level + 1; I am ...
3 years, 7 months ago (2017-05-10 17:25:48 UTC) #52
Zhenyao Mo
lgtm
3 years, 7 months ago (2017-05-12 17:54:07 UTC) #69
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2866663002/140001
3 years, 7 months ago (2017-05-15 05:34:41 UTC) #76
commit-bot: I haz the power
3 years, 7 months ago (2017-05-15 05:40:20 UTC) #79
Message was sent while issue was closed.
Committed patchset #6 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/2e20e662edabca0927a210008507...

Powered by Google App Engine
This is Rietveld 408576698