Skip to content
New issue

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

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

Already on GitHub? # to your account

Add Texture2D.GetDataPointerEXT #486

Merged
merged 2 commits into from
Dec 22, 2024
Merged

Add Texture2D.GetDataPointerEXT #486

merged 2 commits into from
Dec 22, 2024

Conversation

kg
Copy link
Contributor

@kg kg commented Apr 13, 2024

Without this, it's not possible to cleanly queue up texture readback operations to execute later, since GetData checks the size of T against the texture format, instead of checking the size of the buffer - so you can't read a Color texture into a byte[] even if it's big enough.

@flibitijibibo
Copy link
Member

I might be misremembering but I thought this worked like SetData where byte[] was okay... are we just missing some math checks that exist in SetData?

@kg
Copy link
Contributor Author

kg commented Apr 13, 2024

I might be misremembering but I thought this worked like SetData where byte[] was okay... are we just missing some math checks that exist in SetData?

The

	int elementSizeInBytes = MarshalHelper.SizeOf<T>();
	ValidateGetDataFormat(Format, elementSizeInBytes);

should reject an attempt to download a Color surface into a byte[], I think. IIRC the last time I tried it, it didn't work.

@kg
Copy link
Contributor Author

kg commented Aug 30, 2024

Alternate proposal: Don't do this, instead we add GetDataPointerAsyncEXT/SetDataPointerAsyncEXT, which only work on the SDL_GPU backend. If we had those I'd use them instead

@kg
Copy link
Contributor Author

kg commented Dec 20, 2024

Is there anything I could do to land this one?

@flibitijibibo
Copy link
Member

Meant to check this but never got around to it... I'm still pretty sure this works; the format size of any SurfaceFormat % sizeof(byte) should always be 0, shouldn't it? A small test that just gets a blank 16x16 Color texture should be able to verify this.

@kg
Copy link
Contributor Author

kg commented Dec 20, 2024

It does appear to be working now, I wonder why I couldn't get it to work before... the generic instances for GetData are still a little annoying to work around though, and it would be nice to be able to use a buffer outside of the managed heap. Will think it over a bit.

@kg
Copy link
Contributor Author

kg commented Dec 22, 2024

So the use case is this scenario, where I queue up readbacks to happen at the 'right time'. Is there an obvious solution for this I'm missing other than the awful cascade of elseifs and casts? You can see what this PR allows in the else block at the end.

if (rb.Destination is byte[] ba) {
    rb.Source.GetData(0, null, ba, 0, ba.Length);
} else if (rb.Destination is ushort[] ua) {
    rb.Source.GetData(0, null, ua, 0, ua.Length);
} else if (/* ... */) {
    // ...
} else {
    var gch = GCHandle.Alloc(rb.Destination, GCHandleType.Pinned);
    try {
        var size = Marshal.SizeOf(rb.Destination.GetType().GetElementType()) * rb.Destination.Length;
        rb.Source.GetDataPointerEXT(0, null, gch.AddrOfPinnedObject(), size);
    } finally {
        gch.Free();
    }
}

@flibitijibibo
Copy link
Member

I guess the weird part is why rb.Destination is ambiguous like that... is this a case of cascading templates or something? Are the templates super important in some way?

@kg
Copy link
Contributor Author

kg commented Dec 22, 2024

rb.Destination is an Array, since i just have a list of readbacks queued up. I guess I could create generic instances with virtual methods, and that's probably not bad.

Copy link
Member

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

Looked around the Graphics namespace and I think this is the only qualifying class to need GetDataPointer since buffers never need it and 3D wouldn't support it anyway - I'd hate to see a renderer that needs readback on cubes.

Once the whitespace is fixed we'll go ahead with this one.

Co-authored-by: Ethan Lee <flibitijibibo@gmail.com>
@flibitijibibo flibitijibibo merged commit ebadfa6 into FNA-XNA:master Dec 22, 2024
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants