GLES Mid-Execution Capture #2016
Conversation
Android drivers love to call eglGetError from within other egl* functions.
| @@ -440,17 +440,6 @@ public AndroidInput( | |||
| traceTarget.addBoxListener(SWT.Modify, targetListener); | |||
| targetListener.handleEvent(null); | |||
|
|
|||
| Listener apiListener = e -> { | |||
There was a problem hiding this comment.
Are we confident enough marking this as "works" for GLES, or do we want to mark it as experimental?
There was a problem hiding this comment.
Because as soon as the button works, people will start sending bugs.
There was a problem hiding this comment.
Looks good to me.
I would like @ben-clayton to take a look at the GLES stuff which I am a bit out of practice on.
gapis/api/gles/state_builder.go
Outdated
| @@ -201,6 +201,10 @@ func (sb *stateBuilder) contextObject(ctx context.Context, handle EGLContext, c | |||
| if !c.Other().Initialized() { | |||
| return | |||
| } | |||
| if c.Other().Destroyed() { | |||
There was a problem hiding this comment.
Not too sure what the point is in creating a context just to delete it again.
Seems a bit excessive for the correctness argument.
gapis/api/gles/externs.go
Outdated
| @@ -61,15 +61,15 @@ func (e externs) unmapMemory(slice memory.Slice) { | |||
| } | |||
|
|
|||
| func (e externs) GetEGLStaticContextState(EGLDisplay, EGLContext) StaticContextStateʳ { | |||
| return FindStaticContextState(e.s.Arena, e.cmd.Extras()).Clone(e.s.Arena, api.CloneContext{}) | |||
| return FindStaticContextState(e.s.Arena, e.cmd.Extras()) | |||
There was a problem hiding this comment.
Not sure about this change. "No need to clone what is already a copy." Where is this deep copy of which you speak?
| case false: make!u8(size) | ||
| b.Data = make!u8(size) | ||
| if (data != null) { | ||
| copy(b.Data, as!u8*(data)[0:size]) |
There was a problem hiding this comment.
What was the reason for this change?
There was a problem hiding this comment.
@spy_disabled and clone do not get along well, causing the observation to be dropped. copy on the other hand works fine.
There was a problem hiding this comment.
I'm working on this stuff with the new compiler. Care to elaborate a little? Maybe I can get this fixed.
There was a problem hiding this comment.
@spy_disabled is not really fully implemented. It is partially handled in a few places In the templates. One of the problems is that when evaluating an assignment, if the LHS is @spy_disabled, the RHS is not evaluated. This is why foo = clone(bar...) drops the observation on bar if foo is @spy_disabled as the clone is not evaluated. Simply evaluating the RHS will lead to other problems, as it would have to be evaluated in a @spy_disabled way, for example not issuing an error (maybe) if reading from another @spy_disabled source - say from bar in the clone - it's OK to copy from @spy_disabled to @spy_disabled, but not to copy from @spy_disabled to not-@spy_disabled since the data would not be available at trace time.
It seemed overly complex to me to get assignment to behave correctly, and too hacky to add special handling to "assignment with clone RHS", so I did what Vulkan does and dropped the clone shortcut. For tracing it doesn't matter, since the b = make!u8(size) gets dropped and the copy is only used to create the observation. I didn't think the clone is all that much faster during mutate than make followed by copy.
gapis/api/gles/state_builder.go
Outdated
| // Get the largest used shader ID. | ||
| maxID := uint32(0) | ||
| if l := c.Objects().Shaders().Len(); l > 0 { | ||
| maxID = uint32(c.Objects().Shaders().Keys()[l-1]) |
There was a problem hiding this comment.
Some assumptions here about sequential keys which I'm not sure is guaranteed. Would iterating over all shaders to take the maximum id field really too slow?
There was a problem hiding this comment.
Done. I knew keys are sorted, but thought .Keys() was constant time...
| case false: make!u8(size) | ||
| b.Data = make!u8(size) | ||
| if (data != null) { | ||
| copy(b.Data, as!u8*(data)[0:size]) |
gapis/api/gles/state_builder.go
Outdated
| @@ -63,6 +65,11 @@ func (s *State) RebuildState(ctx context.Context, oldState *api.GlobalState) ([] | |||
| representative := map[ShareListʳ]EGLContext{} | |||
| for i := ContextID(0); i < s.NextContextID(); i++ { | |||
| for handle, c := range s.EGLContexts().All() { | |||
| // Don't recreate destroyed or unitialized contexts. | |||
- Don't put references into the extras in MEC state rebuilding. - Clone extras before using them in the new state. - Handle null context extras.
Introduces mechanisms to read back data for renderbuffers and textures from the GPU for MEC.
Use the shaders and their source as they were when the program was linked, not using the program state on serialization. Shaders can be detached, have their source change, and even deleted, while the program that they were used to link stays unaffected.
Still missing lots of pieces, but works on some stuff.
For #1197