Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framebuffer view does not always match thumbnail for groups #1134

Closed
ben-clayton opened this issue Sep 25, 2017 · 7 comments
Closed

Framebuffer view does not always match thumbnail for groups #1134

ben-clayton opened this issue Sep 25, 2017 · 7 comments

Comments

@ben-clayton
Copy link
Contributor

We now do 'smart' analysis to show a sensible thumbnail for a group in the command tree.
Clicking on that group will always show the framebuffer after the last command in the group.

For development Unity builds of GVR applications, the frame group ends with a non-scene framebuffer attached leading to the undefined-framebuffer pattern being shown.

We should probably just change the rules so that the command selection for a group matches the thumbnail.

@ben-clayton ben-clayton added this to the V1.0 milestone Sep 25, 2017
@pmuetschard
Copy link
Member

I propose we change the getFramebufferAttachment API to use the following request:

message GetFramebufferAttachmentRequest {
  path.Device device = 1;
  api.FramebufferAttachment attachment = 2;
  RenderSettings settings = 3;
  UsageHints hints = 4;

  oneof after {
    path.Command command = 5;
    path.CommandTreeNode command_tree_node = 6;
  }
}

What do you think?

@ben-clayton
Copy link
Contributor Author

Not a bad idea. Let me investigate the amount of wiring needed to implement this.

@ben-clayton
Copy link
Contributor Author

I got most the way through implementing this, and then suddenly wondered:

Does it make sense to have the framebuffer view show something mid-way through the group, but have everything else (state, memory, textures, etc) show the state at the end of the group?

What are your thoughts?

@dsrbecky
Copy link
Contributor

Fair point, it would make sense to sync everything else to match the point at framebuffer.

In my mind, a group node should have one field which is "the representative command", and we should get the thumbnail using that. Same goes for framebuffer. And now that you mention it, it would make sense to apply it to all other state as well.

@ben-clayton
Copy link
Contributor Author

Thanks @dsrbecky. I kinda agree, but I'm also troubled with the idea of the state not being at the end when clicking on a group or a command with children.

@AWoloszyn, @pmuetschard - what are your thoughts?

@AWoloszyn
Copy link
Contributor

AWoloszyn commented Oct 18, 2017

Sorry, I thought I had already responded.

My #1 concern would be that everything matches. It would be terrible if the State != What created the framebuffer.

That being said, I also like the idea of that being the last command in the group, but I totally understand that in some cases, the "last command" is not actually a useful piece of data to be visualizing.

Would doing something "simple" like a small area that says WHAT command this is the data for be sufficient. (Somewhere global-ish, that is always visible), then we can at least know WHICH command the thumbnail/framebuffer is for.

@pmuetschard
Copy link
Member

I suppose we're back to my original suggestion from the proto-conversion time where tree nodes contain a Path.Command to the command that is most representative of the group. Using that path to get the FB, state, textures, etc. make sense to me. I think the same arguments that apply to choosing a representative command for the thumbnail applies here. After all, we only choose a different command than the last in the group, if we think it makes more sense.

We could show the current selection in the currently empty status bar.

ben-clayton added a commit to ben-clayton/gapid that referenced this issue Oct 23, 2017
Use this for picking a command to show data for when selecting a group.

Fixes: google#1134
ben-clayton added a commit to ben-clayton/gapid that referenced this issue Oct 23, 2017
Use this for picking a command to show data for when selecting a group.

Fixes: google#1134
ben-clayton added a commit to ben-clayton/gapid that referenced this issue Oct 23, 2017
Use this for picking a command to show data for when selecting a group.

Fixes: google#1134
ben-clayton added a commit that referenced this issue Oct 24, 2017
Use this for picking a command to show data for when selecting a group.

Fixes: #1134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants