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

add texture array support to vulkan readback #3235

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pgruenbacher
Copy link
Contributor

add in numLayers to ReadBackVK utility class so that it properly copies texture arrays when called by bgfx::readTexture.

tested in examples-20-picking to still work

The ReadBackVK class could probably be made to be more elegant as well as to handle texturecube but I don't have a good test-case for that.

…es texture arrays when called by bgfx::readTexture.
@bkaradzic
Copy link
Owner

cc @pezcode

Copy link
Contributor

@pezcode pezcode left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the one suggested change 👍

Comment on lines 5667 to 5668
, _mip
, 1
, 0
, 1
);
Copy link
Contributor

Choose a reason for hiding this comment

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

The first 1 (_levelCount) should still be there, to only add a layout transition for _mip. Otherwise the driver will transition all higher mip levels as well.

Same up top.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep agreed. i changed it back to level=1, baseArrayLayer=1, layerCount = VK_REMAINING_ARRAY_LAYERS for imageMemoryBarrier

@bkaradzic
Copy link
Owner

Make sure this have the same behavior on other backends too.

@pgruenbacher
Copy link
Contributor Author

Make sure this have the same behavior on other backends too.

it should for opengl

looking at metal it seems to be the same behavior
more importantly since readTexture only accepts mip as an argument, the expected behavior should be to copy all of the data and not just a single slice of the array. otherwise no other way to get the data

@bkaradzic
Copy link
Owner

more importantly since readTexture only accepts mip as an argument

I think maybe API should change... Intention was always to read single 2D slice, not complete chain. Something like:

	uint32_t readTexture(
		  TextureHandle _handle
		, void* _data
		, uint16_t _layer
		, uint8_t _mip
		);

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