Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2342063002:
simple fix? (Closed)
|
Created:
4 years, 3 months ago by mtklein_C Modified:
4 years, 3 months ago Reviewers:
csmartdalton CC:
reviews_skia.org Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
Descriptionsimple fix?
Scope cmdInfo more tightly to where it's not a null reference.
BUG=skia:5759
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002
Committed: https://skia.googlesource.com/skia/+/35b26a457100804b7a782cdff1132d2b65176c35
Patch Set 1 #
Total comments: 1
Patch Set 2 : alt #Messages
Total messages: 19 (11 generated)
Description was changed from ========== simple fix? Scope cmdInfo more tightly. BUG=skia:5759 ========== to ========== simple fix? Scope cmdInfo more tightly. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 ==========
Description was changed from ========== simple fix? Scope cmdInfo more tightly. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 ========== to ========== simple fix? Scope cmdInfo more tightly. cmdInfo is only a sound reference when !fDrawIndirectBuffer. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 ==========
mtklein@chromium.org changed reviewers: + csmartdalton@google.com
The CQ bit was checked by mtklein@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Note for Reviewers: The CQ is waiting for an approval. If you believe that the CL is not ready yet, or if you would like to L-G-T-M with comments then please uncheck the CQ checkbox. Waiting for LGTM from valid reviewer(s) till 2016-09-15 21:44 UTC
https://codereview.chromium.org/2342063002/diff/1/src/gpu/instanced/GLInstanc... File src/gpu/instanced/GLInstancedRendering.cpp (left): https://codereview.chromium.org/2342063002/diff/1/src/gpu/instanced/GLInstanc... src/gpu/instanced/GLInstancedRendering.cpp:257: emulatedBaseInstance += cmdInfo.fInstanceCount; This needs to happen for the glDrawElementsIndirect case as well, if you don't have EXT_base_instance. You can thank OpenGL ES for making this so confusing and adding an extension that lets them ignore a field in the DrawIndirect s5ruct :(
The CQ bit was unchecked by csmartdalton@google.com
On 2016/09/15 at 16:16:24, csmartdalton wrote: > https://codereview.chromium.org/2342063002/diff/1/src/gpu/instanced/GLInstanc... > File src/gpu/instanced/GLInstancedRendering.cpp (left): > > https://codereview.chromium.org/2342063002/diff/1/src/gpu/instanced/GLInstanc... > src/gpu/instanced/GLInstancedRendering.cpp:257: emulatedBaseInstance += cmdInfo.fInstanceCount; > This needs to happen for the glDrawElementsIndirect case as well, if you don't have EXT_base_instance. > > You can thank OpenGL ES for making this so confusing and adding an extension that lets them ignore a field in the DrawIndirect s5ruct :( Like this?
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== simple fix? Scope cmdInfo more tightly. cmdInfo is only a sound reference when !fDrawIndirectBuffer. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 ========== to ========== simple fix? Scope cmdInfo more tightly to where it's not a null reference. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 ==========
On 2016/09/15 16:25:28, mtklein_C wrote: > On 2016/09/15 at 16:16:24, csmartdalton wrote: > > > https://codereview.chromium.org/2342063002/diff/1/src/gpu/instanced/GLInstanc... > > File src/gpu/instanced/GLInstancedRendering.cpp (left): > > > > > https://codereview.chromium.org/2342063002/diff/1/src/gpu/instanced/GLInstanc... > > src/gpu/instanced/GLInstancedRendering.cpp:257: emulatedBaseInstance += > cmdInfo.fInstanceCount; > > This needs to happen for the glDrawElementsIndirect case as well, if you don't > have EXT_base_instance. > > > > You can thank OpenGL ES for making this so confusing and adding an extension > that lets them ignore a field in the DrawIndirect s5ruct :( > > Like this? Yep, lgtm Or we can just skip the ref completely and index into the array on the spot. I don't really care either way. Nice catch
The CQ bit was unchecked by mtklein@chromium.org
The CQ bit was checked by mtklein@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== simple fix? Scope cmdInfo more tightly to where it's not a null reference. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 ========== to ========== simple fix? Scope cmdInfo more tightly to where it's not a null reference. BUG=skia:5759 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342063002 Committed: https://skia.googlesource.com/skia/+/35b26a457100804b7a782cdff1132d2b65176c35 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/35b26a457100804b7a782cdff1132d2b65176c35 |
