Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
509723002:
Added support for GPU Tracing on mobile devices which support it. (Closed)
DescriptionAdded support for GPU Tracing on mobile devices which support it.
Newer mobile devices may support the Open GL extension
EXT_disjoint_timer_query, these devices can now be traced in Chrome.
R=vmiura@chromium.org
BUG= https://code.google.com/p/chromium/issues/detail?id=397294
TEST= trybots
Committed: https://crrev.com/882e1b72498ddeaa0236b03f26710783b36df20c
Cr-Commit-Position: refs/heads/master@{#293150}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Fixed unit test #
Total comments: 4
Patch Set 3 : Replaced DiscardTrace() with simply clear(), rely on destructor to always delete trace queries. #Patch Set 4 : Merged ARB/EXT variants for glGenQueries, glDeleteQueries, and glQueryCounter #Patch Set 5 : Fixed unit tests, added unit tests for disjoint timer trace #Patch Set 6 : Do not collapse genQueries with genQueriesARB/EXT #Patch Set 7 : Combined glGetQueryObject ARB/EXT calls #Patch Set 8 : removed gl ARB calls which don't exist #Patch Set 9 : Fixed unit tests #Patch Set 10 : Fixed another gpu tracer unittest issue #
Total comments: 10
Patch Set 11 : Simplified and collapsed glGetQueryObject bindings #
Messages
Total messages: 24 (3 generated)
vmiura@chromium.org changed reviewers: + piman@chromium.org
+piman for ui/ changes
https://codereview.chromium.org/509723002/diff/20001/gpu/command_buffer/servi... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/20001/gpu/command_buffer/servi... gpu/command_buffer/service/gpu_tracer.cc:173: GLuint64 beg_stamp = 0; nit: no abbreviations. begin_stamp? https://codereview.chromium.org/509723002/diff/20001/ui/gl/generate_bindings.py File ui/gl/generate_bindings.py (right): https://codereview.chromium.org/509723002/diff/20001/ui/gl/generate_bindings.... ui/gl/generate_bindings.py:184: 'arguments': 'GLsizei n, const GLuint* ids', }, Why did you separate the ARB version from the EXT version? The bindings should fallback to the right implementation depending on the extensions availability. I.e. if you specify: 'names': ['glDeleteQueriesARB', 'glDeleteQueriesEXT'] Then you can call glDeleteQueriesARB in chromium code, and that will call glDeleteQueriesEXT in the driver if that's what's available. That avoids a large number of unnecessary if tests in chromium code, since both calls are meant to be equivalent.
Looks mostly good. Could we unit test kTracerTypeDisjointTimer case, for normal behavior and disjoint event behavior? https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... File gpu/command_buffer/service/gpu_tracer.cc (left): https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gpu_tracer.cc:170: gpu_timing_synced_ = false; I think we still need this part, so that we sync timing each time we start tracing. https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gpu_tracer.cc:183: glDeleteQueries(2, queries_); ~GPUTrace deletes the queries too so perhaps we should remove these deletes. https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gpu_tracer.cc:378: traces_.pop_front(); Can we completely remove DiscardTrace() & Process() calls? The destructor will delete the queries on traces_.pop_front(). That removes the need for 'discard_trace_' also.
https://codereview.chromium.org/509723002/diff/20001/gpu/command_buffer/servi... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/20001/gpu/command_buffer/servi... gpu/command_buffer/service/gpu_tracer.cc:173: GLuint64 beg_stamp = 0; On 2014/08/26 23:43:57, piman (OOO) wrote: > nit: no abbreviations. begin_stamp? This was shortened to avoid multiple lines for some of the expressions below, but I'll just change it back to begin_stamp as you suggested. https://codereview.chromium.org/509723002/diff/20001/ui/gl/generate_bindings.py File ui/gl/generate_bindings.py (right): https://codereview.chromium.org/509723002/diff/20001/ui/gl/generate_bindings.... ui/gl/generate_bindings.py:184: 'arguments': 'GLsizei n, const GLuint* ids', }, On 2014/08/26 23:43:57, piman (OOO) wrote: > Why did you separate the ARB version from the EXT version? > > The bindings should fallback to the right implementation depending on the > extensions availability. I.e. if you specify: > 'names': ['glDeleteQueriesARB', 'glDeleteQueriesEXT'] > > Then you can call glDeleteQueriesARB in chromium code, and that will call > glDeleteQueriesEXT in the driver if that's what's available. > > That avoids a large number of unnecessary if tests in chromium code, since both > calls are meant to be equivalent. Does that mean I should be calling glDeleteQueriesARB() when I intend to call glDeleteQueriesEXT()? or should there be a separate entry just for glDeleteQueriesEXT() as well?
On Wed, Aug 27, 2014 at 10:03 AM, <dyen@chromium.org> wrote: > > https://codereview.chromium.org/509723002/diff/20001/gpu/ > command_buffer/service/gpu_tracer.cc > File gpu/command_buffer/service/gpu_tracer.cc (right): > > https://codereview.chromium.org/509723002/diff/20001/gpu/ > command_buffer/service/gpu_tracer.cc#newcode173 > gpu/command_buffer/service/gpu_tracer.cc:173: GLuint64 beg_stamp = 0; > On 2014/08/26 23:43:57, piman (OOO) wrote: > >> nit: no abbreviations. begin_stamp? >> > > This was shortened to avoid multiple lines for some of the expressions > below, but I'll just change it back to begin_stamp as you suggested. > > > https://codereview.chromium.org/509723002/diff/20001/ui/ > gl/generate_bindings.py > File ui/gl/generate_bindings.py (right): > > https://codereview.chromium.org/509723002/diff/20001/ui/ > gl/generate_bindings.py#newcode184 > ui/gl/generate_bindings.py:184: 'arguments': 'GLsizei n, const GLuint* > ids', }, > On 2014/08/26 23:43:57, piman (OOO) wrote: > >> Why did you separate the ARB version from the EXT version? >> > > The bindings should fallback to the right implementation depending on >> > the > >> extensions availability. I.e. if you specify: >> 'names': ['glDeleteQueriesARB', 'glDeleteQueriesEXT'] >> > > Then you can call glDeleteQueriesARB in chromium code, and that will >> > call > >> glDeleteQueriesEXT in the driver if that's what's available. >> > > That avoids a large number of unnecessary if tests in chromium code, >> > since both > >> calls are meant to be equivalent. >> > > Does that mean I should be calling glDeleteQueriesARB() when I intend to > call glDeleteQueriesEXT()? Yes. glDeleteQueriesEXT/glDeleteQueriesARB/glDeleteQueries are all equivalent. That's true of most other variations of any GL call (there's exceptions, but these are not). or should there be a separate entry just for > glDeleteQueriesEXT() as well? > > https://codereview.chromium.org/509723002/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
I will add some unit tests after I sort out which GL functions to call. https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... File gpu/command_buffer/service/gpu_tracer.cc (left): https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gpu_tracer.cc:170: gpu_timing_synced_ = false; On 2014/08/26 23:47:26, vmiura wrote: > I think we still need this part, so that we sync timing each time we start > tracing. Okay good point. I've moved this to the CalculateTimerOffset() function to localize all the usage of gpu_timing_synced_. https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gpu_tracer.cc:183: glDeleteQueries(2, queries_); On 2014/08/26 23:47:26, vmiura wrote: > ~GPUTrace deletes the queries too so perhaps we should remove these deletes. Done. https://codereview.chromium.org/509723002/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gpu_tracer.cc:378: traces_.pop_front(); On 2014/08/26 23:47:26, vmiura wrote: > Can we completely remove DiscardTrace() & Process() calls? > The destructor will delete the queries on traces_.pop_front(). > > That removes the need for 'discard_trace_' also. Yes you are right, I think I was thrown off by how the invalid case was done above (line 354). Looking at the code calling Process() is clearly unnecessary. I'll change them both to simply do "traces_.clear();".
On 2014/08/27 18:16:42, piman (OOO) wrote: > On Wed, Aug 27, 2014 at 10:03 AM, <mailto:dyen@chromium.org> wrote: > > > > > https://codereview.chromium.org/509723002/diff/20001/gpu/ > > command_buffer/service/gpu_tracer.cc > > File gpu/command_buffer/service/gpu_tracer.cc (right): > > > > https://codereview.chromium.org/509723002/diff/20001/gpu/ > > command_buffer/service/gpu_tracer.cc#newcode173 > > gpu/command_buffer/service/gpu_tracer.cc:173: GLuint64 beg_stamp = 0; > > On 2014/08/26 23:43:57, piman (OOO) wrote: > > > >> nit: no abbreviations. begin_stamp? > >> > > > > This was shortened to avoid multiple lines for some of the expressions > > below, but I'll just change it back to begin_stamp as you suggested. > > > > > > https://codereview.chromium.org/509723002/diff/20001/ui/ > > gl/generate_bindings.py > > File ui/gl/generate_bindings.py (right): > > > > https://codereview.chromium.org/509723002/diff/20001/ui/ > > gl/generate_bindings.py#newcode184 > > ui/gl/generate_bindings.py:184: 'arguments': 'GLsizei n, const GLuint* > > ids', }, > > On 2014/08/26 23:43:57, piman (OOO) wrote: > > > >> Why did you separate the ARB version from the EXT version? > >> > > > > The bindings should fallback to the right implementation depending on > >> > > the > > > >> extensions availability. I.e. if you specify: > >> 'names': ['glDeleteQueriesARB', 'glDeleteQueriesEXT'] > >> > > > > Then you can call glDeleteQueriesARB in chromium code, and that will > >> > > call > > > >> glDeleteQueriesEXT in the driver if that's what's available. > >> > > > > That avoids a large number of unnecessary if tests in chromium code, > >> > > since both > > > >> calls are meant to be equivalent. > >> > > > > Does that mean I should be calling glDeleteQueriesARB() when I intend to > > call glDeleteQueriesEXT()? > > > Yes. glDeleteQueriesEXT/glDeleteQueriesARB/glDeleteQueries are all > equivalent. That's true of most other variations of any GL call (there's > exceptions, but these are not). > > or should there be a separate entry just for > > glDeleteQueriesEXT() as well? > > > > https://codereview.chromium.org/509723002/ > > > > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. Should we collapse glDeleteQueriesARB into glDeleteQueries as well then? So delete ['glDeleteQueriesARB'] as well and turn ['glDeleteQueries'] into ['glDeleteQueries', 'glDeleteQueriesARB', 'glDeleteQueriesEXT'].
On Wed, Aug 27, 2014 at 12:49 PM, <dyen@chromium.org> wrote: > On 2014/08/27 18:16:42, piman (OOO) wrote: > >> On Wed, Aug 27, 2014 at 10:03 AM, <mailto:dyen@chromium.org> wrote: >> > > > >> > https://codereview.chromium.org/509723002/diff/20001/gpu/ >> > command_buffer/service/gpu_tracer.cc >> > File gpu/command_buffer/service/gpu_tracer.cc (right): >> > >> > https://codereview.chromium.org/509723002/diff/20001/gpu/ >> >> > command_buffer/service/gpu_tracer.cc#newcode173 >> > gpu/command_buffer/service/gpu_tracer.cc:173: GLuint64 beg_stamp = 0; >> > On 2014/08/26 23:43:57, piman (OOO) wrote: >> > >> >> nit: no abbreviations. begin_stamp? >> >> >> > >> > This was shortened to avoid multiple lines for some of the expressions >> > below, but I'll just change it back to begin_stamp as you suggested. >> > >> > >> > https://codereview.chromium.org/509723002/diff/20001/ui/ >> > gl/generate_bindings.py >> > File ui/gl/generate_bindings.py (right): >> > >> > https://codereview.chromium.org/509723002/diff/20001/ui/ >> > gl/generate_bindings.py#newcode184 >> > ui/gl/generate_bindings.py:184: 'arguments': 'GLsizei n, const GLuint* >> > ids', }, >> > On 2014/08/26 23:43:57, piman (OOO) wrote: >> > >> >> Why did you separate the ARB version from the EXT version? >> >> >> > >> > The bindings should fallback to the right implementation depending on >> >> >> > the >> > >> >> extensions availability. I.e. if you specify: >> >> 'names': ['glDeleteQueriesARB', 'glDeleteQueriesEXT'] >> >> >> > >> > Then you can call glDeleteQueriesARB in chromium code, and that will >> >> >> > call >> > >> >> glDeleteQueriesEXT in the driver if that's what's available. >> >> >> > >> > That avoids a large number of unnecessary if tests in chromium code, >> >> >> > since both >> > >> >> calls are meant to be equivalent. >> >> >> > >> > Does that mean I should be calling glDeleteQueriesARB() when I intend to >> > call glDeleteQueriesEXT()? >> > > > Yes. glDeleteQueriesEXT/glDeleteQueriesARB/glDeleteQueries are all >> equivalent. That's true of most other variations of any GL call (there's >> exceptions, but these are not). >> > > or should there be a separate entry just for >> > glDeleteQueriesEXT() as well? >> > >> > https://codereview.chromium.org/509723002/ >> > >> > > To unsubscribe from this group and stop receiving emails from it, send an >> > email > >> to mailto:chromium-reviews+unsubscribe@chromium.org. >> > > Should we collapse glDeleteQueriesARB into glDeleteQueries as well then? So > delete ['glDeleteQueriesARB'] as well and turn ['glDeleteQueries'] into > ['glDeleteQueries', 'glDeleteQueriesARB', 'glDeleteQueriesEXT']. > Yes, I think so. > > https://codereview.chromium.org/509723002/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/08/27 20:39:08, piman (OOO) wrote: > On Wed, Aug 27, 2014 at 12:49 PM, <mailto:dyen@chromium.org> wrote: > > > On 2014/08/27 18:16:42, piman (OOO) wrote: > > > >> On Wed, Aug 27, 2014 at 10:03 AM, <mailto:dyen@chromium.org> wrote: > >> > > > > > > >> > https://codereview.chromium.org/509723002/diff/20001/gpu/ > >> > command_buffer/service/gpu_tracer.cc > >> > File gpu/command_buffer/service/gpu_tracer.cc (right): > >> > > >> > https://codereview.chromium.org/509723002/diff/20001/gpu/ > >> > >> > command_buffer/service/gpu_tracer.cc#newcode173 > >> > gpu/command_buffer/service/gpu_tracer.cc:173: GLuint64 beg_stamp = 0; > >> > On 2014/08/26 23:43:57, piman (OOO) wrote: > >> > > >> >> nit: no abbreviations. begin_stamp? > >> >> > >> > > >> > This was shortened to avoid multiple lines for some of the expressions > >> > below, but I'll just change it back to begin_stamp as you suggested. > >> > > >> > > >> > https://codereview.chromium.org/509723002/diff/20001/ui/ > >> > gl/generate_bindings.py > >> > File ui/gl/generate_bindings.py (right): > >> > > >> > https://codereview.chromium.org/509723002/diff/20001/ui/ > >> > gl/generate_bindings.py#newcode184 > >> > ui/gl/generate_bindings.py:184: 'arguments': 'GLsizei n, const GLuint* > >> > ids', }, > >> > On 2014/08/26 23:43:57, piman (OOO) wrote: > >> > > >> >> Why did you separate the ARB version from the EXT version? > >> >> > >> > > >> > The bindings should fallback to the right implementation depending on > >> >> > >> > the > >> > > >> >> extensions availability. I.e. if you specify: > >> >> 'names': ['glDeleteQueriesARB', 'glDeleteQueriesEXT'] > >> >> > >> > > >> > Then you can call glDeleteQueriesARB in chromium code, and that will > >> >> > >> > call > >> > > >> >> glDeleteQueriesEXT in the driver if that's what's available. > >> >> > >> > > >> > That avoids a large number of unnecessary if tests in chromium code, > >> >> > >> > since both > >> > > >> >> calls are meant to be equivalent. > >> >> > >> > > >> > Does that mean I should be calling glDeleteQueriesARB() when I intend to > >> > call glDeleteQueriesEXT()? > >> > > > > > > Yes. glDeleteQueriesEXT/glDeleteQueriesARB/glDeleteQueries are all > >> equivalent. That's true of most other variations of any GL call (there's > >> exceptions, but these are not). > >> > > > > or should there be a separate entry just for > >> > glDeleteQueriesEXT() as well? > >> > > >> > https://codereview.chromium.org/509723002/ > >> > > >> > > > > To unsubscribe from this group and stop receiving emails from it, send an > >> > > email > > > >> to mailto:chromium-reviews+unsubscribe@chromium.org. > >> > > > > Should we collapse glDeleteQueriesARB into glDeleteQueries as well then? So > > delete ['glDeleteQueriesARB'] as well and turn ['glDeleteQueries'] into > > ['glDeleteQueries', 'glDeleteQueriesARB', 'glDeleteQueriesEXT']. > > > > Yes, I think so. > > > > > > https://codereview.chromium.org/509723002/ > > > > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. After collapsing the glGenQueries calls, it looks like glGenQueriesEXT does not work properly on windows, I am getting an error here: http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered... For what it is worth, it does work on linux: http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger... Should I only collapse glGenQueriesARB and glGenQueriesEXT instead?
https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:109: glQueryCounter(queries_[0], GL_TIMESTAMP_EXT); nit: GL_TIMESTAMP_EXT and GL_TIMESTAMP have the same value, you can collapse the 2 versions. https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:124: glQueryCounter(queries_[1], GL_TIMESTAMP_EXT); nit: same here https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:146: &done); Same here with GL_QUERY_RESULT_AVAILABLE_EXT vs GL_QUERY_RESULT_AVAILABLE https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:179: glGetQueryObjectui64vEXT(queries_[1], GL_QUERY_RESULT_EXT, &end_stamp); nit: can we collapse those as well?
https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:109: glQueryCounter(queries_[0], GL_TIMESTAMP_EXT); On 2014/09/02 18:41:44, piman (OOO) wrote: > nit: GL_TIMESTAMP_EXT and GL_TIMESTAMP have the same value, you can collapse the > 2 versions. I noticed that as well, but I wanted to follow the docs properly in case someone else comes along. Do you think just using GL_TIMESTAMP is the right approach since it's technically the same? https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:179: glGetQueryObjectui64vEXT(queries_[1], GL_QUERY_RESULT_EXT, &end_stamp); On 2014/09/02 18:41:44, piman (OOO) wrote: > nit: can we collapse those as well? When I collapsed these, it actually started to crash to GPU. I will do more testing, maybe something else was happening.
On Tue, Sep 2, 2014 at 11:45 AM, <dyen@chromium.org> wrote: > > https://codereview.chromium.org/509723002/diff/180001/gpu/ > command_buffer/service/gpu_tracer.cc > File gpu/command_buffer/service/gpu_tracer.cc (right): > > https://codereview.chromium.org/509723002/diff/180001/gpu/ > command_buffer/service/gpu_tracer.cc#newcode109 > gpu/command_buffer/service/gpu_tracer.cc:109: > glQueryCounter(queries_[0], GL_TIMESTAMP_EXT); > On 2014/09/02 18:41:44, piman (OOO) wrote: > >> nit: GL_TIMESTAMP_EXT and GL_TIMESTAMP have the same value, you can >> > collapse the > >> 2 versions. >> > > I noticed that as well, but I wanted to follow the docs properly in case > someone else comes along. Do you think just using GL_TIMESTAMP is the > right approach since it's technically the same? I think it's clearer/simpler code. The meaning and value of the enums are the same, I don't think we need to be pedantic about whether we use the extension enum or not. If there are variations, we prefer hiding them in ui/gl/gl_gl_api_implementation.cc, not sprinkled at every call site. You can add a comment if you think that helps. > > > https://codereview.chromium.org/509723002/diff/180001/gpu/ > command_buffer/service/gpu_tracer.cc#newcode179 > gpu/command_buffer/service/gpu_tracer.cc:179: > glGetQueryObjectui64vEXT(queries_[1], GL_QUERY_RESULT_EXT, &end_stamp); > On 2014/09/02 18:41:44, piman (OOO) wrote: > >> nit: can we collapse those as well? >> > > When I collapsed these, it actually started to crash to GPU. I will do > more testing, maybe something else was happening. > > https://codereview.chromium.org/509723002/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... File gpu/command_buffer/service/gpu_tracer.cc (right): https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:109: glQueryCounter(queries_[0], GL_TIMESTAMP_EXT); On 2014/09/02 18:45:36, David Yen wrote: > On 2014/09/02 18:41:44, piman (OOO) wrote: > > nit: GL_TIMESTAMP_EXT and GL_TIMESTAMP have the same value, you can collapse > the > > 2 versions. > > I noticed that as well, but I wanted to follow the docs properly in case someone > else comes along. Do you think just using GL_TIMESTAMP is the right approach > since it's technically the same? Done. https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:124: glQueryCounter(queries_[1], GL_TIMESTAMP_EXT); On 2014/09/02 18:41:44, piman (OOO) wrote: > nit: same here Done. https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:146: &done); On 2014/09/02 18:41:44, piman (OOO) wrote: > Same here with GL_QUERY_RESULT_AVAILABLE_EXT vs GL_QUERY_RESULT_AVAILABLE Done. https://codereview.chromium.org/509723002/diff/180001/gpu/command_buffer/serv... gpu/command_buffer/service/gpu_tracer.cc:179: glGetQueryObjectui64vEXT(queries_[1], GL_QUERY_RESULT_EXT, &end_stamp); On 2014/09/02 18:45:36, David Yen wrote: > On 2014/09/02 18:41:44, piman (OOO) wrote: > > nit: can we collapse those as well? > > When I collapsed these, it actually started to crash to GPU. I will do more > testing, maybe something else was happening. Re-did this and the crashing no longer happens, must have been something else. I've collapsed these now.
LGTM, thank you!
LGTM.
The CQ bit was checked by dyen@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dyen@chromium.org/509723002/200001
The CQ bit was unchecked by dyen@chromium.org
The CQ bit was checked by dyen@chromium.org
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as c82d6befbe5f80991027ae2dad3952ad8ff758eb
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/882e1b72498ddeaa0236b03f26710783b36df20c Cr-Commit-Position: refs/heads/master@{#293150} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
