Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1834983003:
Remove string-getting methods from WebGraphicsContext3D. (Closed)
|
Created:
4 years, 9 months ago by danakj Modified:
4 years, 8 months ago CC:
bajones, blink-reviews, blink-reviews-api_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chrishtr, chromium-reviews, danakj+watch_chromium.org, dcheng, dglazkov+blink, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, Ken Russell (switch to Gerrit), kinuko+watch, pdr+graphicswatchlist_chromium.org, piman+watch_chromium.org, piman, rwlbuis, Stephen Chennney, no sievers Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove string-getting methods from WebGraphicsContext3D.
This adds a GLStringQuery helper class to modules/webgl/ which
allows sharing code between the various strings we query off the
GLES2Interface, and uses that with the raw GLES2Interface to query
these strings instead of using WebGraphicsContext3D.
R=kbr@chromium.org
TBR=
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
Committed: https://crrev.com/0220f42dbc4c23865d289d25256d623bcef3abbb
Cr-Commit-Position: refs/heads/master@{#383569}
Patch Set 1 #
Total comments: 3
Patch Set 2 : getactive: whitespace #Patch Set 3 : getactive: mock #Patch Set 4 : getactive: moremock #Patch Set 5 : getactive: rebase #Patch Set 6 : getactive: stringlength #Patch Set 7 : getactive: pointers #
Total comments: 3
Dependent Patchsets: Messages
Total messages: 43 (21 generated)
Description was changed from ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org BUG=584497 ========== to ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ==========
danakj@chromium.org changed reviewers: + chrishtr@chromium.org
kbr: everything chrishtr: public OWNERS
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/1
https://codereview.chromium.org/1834983003/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp (right): https://codereview.chromium.org/1834983003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:3026: contextGL()->GetProgramiv(programId, GL_ACTIVE_UNIFORM_MAX_LENGTH, &maxNameLength); This pulls the maxNameLength query out of the loop below yay https://codereview.chromium.org/1834983003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:3029: if (maxNameLength == 0) { If 0, we would skip the for loop below, before, and fall down to the "unknown error" gl error. So the message will be different but more useful now? https://codereview.chromium.org/1834983003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:3050: name = name.left(name.length() - 3); The old code had a mix of info.name and name here, but it just replaced the info.name with the local name on this line, and then used it afterward. So I don't think we need to have 2 variables. Please double check my logic.
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/40001
The CQ bit was unchecked by danakj@chromium.org
lgtm
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/60001
On 2016/03/25 22:35:34, commit-bot: I haz the power wrote: > Dry run: CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1834983003/60001 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/1834983003/60001 lgtm
Description was changed from ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ========== to ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org TBR= BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ==========
TBRing the code deletes in gpu/
The CQ bit was unchecked by danakj@chromium.org
The CQ bit was checked by danakj@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1834983003/#ps60001 (title: "getactive: moremock")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Ah.. looks like I did something wrong here, some test failures. Will look next week then!
piman@chromium.org changed reviewers: + piman@chromium.org
lgtm
Oh I think I see what I did. Fixes in PS6. I was getting the max string length then reading into it and returning that as a WTF::String. But I need to take a substring() down to the length of the actual things read.
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by danakj@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org, zmo@chromium.org, piman@chromium.org Link to the patchset: https://codereview.chromium.org/1834983003/#ps120001 (title: "getactive: pointers")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834983003/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834983003/120001
Message was sent while issue was closed.
Description was changed from ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org TBR= BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ========== to ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org TBR= BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org TBR= BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ========== to ========== Remove string-getting methods from WebGraphicsContext3D. This adds a GLStringQuery helper class to modules/webgl/ which allows sharing code between the various strings we query off the GLES2Interface, and uses that with the raw GLES2Interface to query these strings instead of using WebGraphicsContext3D. R=kbr@chromium.org TBR= BUG=584497 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel Committed: https://crrev.com/0220f42dbc4c23865d289d25256d623bcef3abbb Cr-Commit-Position: refs/heads/master@{#383569} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/0220f42dbc4c23865d289d25256d623bcef3abbb Cr-Commit-Position: refs/heads/master@{#383569}
Message was sent while issue was closed.
esprehn@chromium.org changed reviewers: + esprehn@chromium.org, zmo@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1834983003/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/webgl/GLStringQuery.h (right): https://codereview.chromium.org/1834983003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/webgl/GLStringQuery.h:57: return WTF::emptyString(); you don't need to write WTF:: on all these. https://codereview.chromium.org/1834983003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/webgl/GLStringQuery.h:59: RefPtr<WTF::StringImpl> nameImpl = WTF::StringImpl::createUninitialized(length, logPtr); ditto, don't write WTF::
Message was sent while issue was closed.
https://codereview.chromium.org/1834983003/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/webgl/GLStringQuery.h (right): https://codereview.chromium.org/1834983003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/webgl/GLStringQuery.h:57: return WTF::emptyString(); On 2016/03/28 21:44:15, esprehn wrote: > you don't need to write WTF:: on all these. Oh that stack of using's at the bottom of WTFString.h. ok thanks. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
