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

Added subcommands to resource view. #779

Merged
merged 1 commit into from
Jul 24, 2017

Conversation

AWoloszyn
Copy link
Contributor

This allows us to get textures from subcommands.

@AWoloszyn AWoloszyn requested review from Qining and ben-clayton July 24, 2017 18:47
@@ -60,6 +61,20 @@ func (e *CmdExtras) Add(es ...CmdExtra) {
}
}

// CloneFromOrPanic clones on or more CmdExtras to the list of CmdExtras,
// if there was an error, a panic is raised
func (e *CmdExtras) CloneFromOrPanic(es ...CmdExtra) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've used MustXXX() for the no-error form of XXX() methods elsewhere

@@ -312,7 +312,7 @@ func cutCommandBuffer(ctx context.Context, id api.CmdID,
// idx[3] is the secondary command buffer index inside a vkCmdExecuteCommands
// idx[4] is the secondary command inside the secondary command-buffer
submitCopy := cb.VkQueueSubmit(a.Queue, a.SubmitCount, a.PSubmits, a.Fence, a.Result)
submitCopy.Extras().Add(a.Extras().All()...)
submitCopy.Extras().CloneFromOrPanic(a.Extras().All()...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just: submitCopy := deep.MustClone(a).(*VkQueueSubmit) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason is that we use Add(a.Extras().All()....) A lot elsewhere, when we are not cloning an atom.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not both?

s, err := capture.NewState(ctx)
if err != nil {
return nil, err
}
for _, cmd := range cmds {
log.W(ctx, "State: %+v", cmd)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I missed that one.

@Qining
Copy link
Contributor

Qining commented Jul 24, 2017

LGTM

@AWoloszyn AWoloszyn force-pushed the subcommand-resources branch from 91aafb0 to 887c422 Compare July 24, 2017 19:14
@@ -60,6 +61,20 @@ func (e *CmdExtras) Add(es ...CmdExtra) {
}
}

// MustClone clones on or more CmdExtras to the list of CmdExtras,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one or more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@ben-clayton ben-clayton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to submit as-is, or by adding the deep.MustClone method.

This allows us to get textures from subcommands.
@AWoloszyn AWoloszyn force-pushed the subcommand-resources branch from 887c422 to 0020561 Compare July 24, 2017 19:23
@AWoloszyn AWoloszyn merged commit 774ec8f into google:master Jul 24, 2017
@AWoloszyn AWoloszyn deleted the subcommand-resources branch July 24, 2017 19:23
purvisa-at-google-com pushed a commit that referenced this pull request Sep 29, 2022
Fix ETC converter initialisation

Image Format conversion in AGI done by the converters that are
registered in init time of Go Packages. There are details about this
but simply init function in go packages runs before main when the
packages are loaded.

Recently #651 moved ETC conversion to its own packages similar to ASTC
but different from ASTC, image creation remained in the parent package.
As all the conversion functions called via register map that has registered
during the init, this causes no function from the etc package, which ended up
with it's not being loaded therefore no converter registered.

This is a workaround that just create translator functions to reflect the image
create functions in the parent package and uses those functions in Vulkan/resources
to ensure the package is initialised.

This is suboptimal and only a workaround. I am creating another PR with a refactor
of the image formats. This is only a workaround until the other one can be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants