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

Texture API issues #4290

Open
slimbuck opened this issue May 31, 2022 · 8 comments
Open

Texture API issues #4290

slimbuck opened this issue May 31, 2022 · 8 comments
Assignees
Labels
area: graphics Graphics related issue feature request

Comments

@slimbuck
Copy link
Member

Some observations of the current texture API:

  • setSource only works with 'browser interface' objects (not arrays of data) and the function correctly tracks which mipmap levels are dirty using _levelsUpdated.
  • getSource will return browser interface objects and also arrays of data which it strictly probably shouldn't (since it is matched with setSource).
  • lock and unlock work with arrays data and the entire texture is always re-uploaded if any mipmap level is locked, whether for read or write.

Ideally we'd have an API that:

  • has a single way of updating both types of texture data
  • tracks which face/mipmap levels have been updated and only upload those to GPU
  • makes it simple to get, set and dirtify individual faces/levels

Also, the current API assumes textures have CPU-side data which is uploaded to GPU. However we often have GPU-side texture data which we want to copy back to CPU.

@mvaligursky are there any additional restrictions or requirements imposed by webgpu on texture handling?

@mvaligursky
Copy link
Contributor

Not at the moment, but here are some best practices for WebGPU: https://github.com/toji/webgpu-best-practices/blob/main/img-textures.md

@yaustar
Copy link
Contributor

yaustar commented Sep 29, 2022

From forum: https://forum.playcanvas.com/t/pre-uploading-of-pc-texture/3212/

Users are requesting the ability to manually upload textures to the GPU so they can control the potential framerate stutter when loading in sections of an open world. Eg, they could upload textures over several frames.

@ArztSamuel
Copy link

Eg, they could upload textures over several frames.

To add to that: currently the textures are loaded "on demand" (i.e. when they are first drawn). We would like to be able to upload large textures during our initial loading screen instead, in order to prevent frame stutters later in the game. We would be fine with all textures being uploaded in 'one frame' in that case. It's just the overall timing of that frame we are concerned with.

@LeXXik
Copy link
Contributor

LeXXik commented Mar 1, 2024

Regarding the timing - we work around it by drawing a bunch of cubes with all materials that will be used in game during the first frame (which is hidden by a loading screen, so the player doesn't see them), then remove them and show the game.

@heretique
Copy link
Contributor

heretique commented May 22, 2024

Hi everyone, I recently started looking into adding support to update particular textures/slices from a Texture2DArray. In it's current iteration texture 2d array support is not flexible enough and not that useful. While investigating this I was reading all the discussions from #6004, this ticket and the references mentioned here.
I was also considering additional use cases:

Let's look at texture 2d arrays. They could be used to load a sprite flipbook upfront but it can also be useful to construct atlases dynamically. Those can be used to render terrain or props more optimally (minimizing draw calls, etc.) or used for virtual texturing. These don't have to maintain the CPU side buffers, instead each texture/slice could be uploaded on demand.

Another use case would be for a lightmap atlas or AO atlas for a chunked terrain/scene. We would know how many chunks we have in advance, hence how many textures/slices we need in the texture array and reserve that space with no upfront available data (maybe initialize it with sensible defaults?), so we can start using the texture right away. Over a number of frames we could upload the slice for each chunk.

Consider discarding of CPU side data for reducing memory usage. If I'm not mistaken at the moment PC preserves the texture data on the CPU side, most probably to quickly recover from context lost events, but that duplicates the memory needed for textures (RAM + VRAM, or in case of most mobile devices 2XRAM because RAM is shared with GPU IIRC).
One solution for recovery is to leave this to the users of the engine, if someone chooses to discard CPU side data we consider that the user knows what he's doing and let it be his responsibility to handle recovery of texture data in case of context loss. The engine should only recreate the GPU side textures with reserved space so that it doesn't crash when recovering from context loss.
A more complex solution that can be added later, the engine could keep some sort of connection with the asset that created the texture (probably the majority of textures would be asset created) and if the user opted for discarding the CPU side data it could provide a mechanism of triggering auto-reloading the texture assets (if most of them are cached by the browser it should be fast) and uploading texture data to GPU.

With all the above we should target an API that:
(reiterating @slimbuck's points)

  • has a single way of updating both types of texture data
  • tracks which face/mipmap levels have been updated and only upload those to GPU
  • makes it simple to get, set and dirtify individual faces/levels

(additionally)

  • keeps the API as closely as possible to current one to avoid breaking changes (or at least minimize them)
  • allows option of reserving texture memory on GPU side with no upfront data at construction (useful for textures that are updated over multiple frames)
  • allows option to discard CPU side buffer(s)
  • allows option to immediately upload texture data to GPU on demand (full or partial)

Let me know if any of this makes sense or if anyone has additional use cases or thoughts on how the API should look like.
Should we keep setSource/getSource or get rid of them and just provide a sensible API for lock / unlock that handles all the get/set cases?
From the top of my mind a way to get rid of setSource/getSource would be to make the lock function return additional object types, ImageReader and ImageWriter, besides the usual typed arrays, objects that could handle reading/writing to/from HTMLCanvasElement, HTMLImageElement, HTMLVideoElement or ImageBitmap.

Tagging @liamdon as this relates to #6004.

@heretique
Copy link
Contributor

I worked on some of the changes mentioned above when time allowed for the last couple of weeks.
My main goal was to provide texture updates for single slices of 2D texture arrays with as little changes as possible to the current API but I couldn't find a good compromise that fits all types of textures. If I try to minimize the number of API changes the internals become more messy, for example the private property _levels needs to become a Map<number, Map<number, Uint8Array>> to be able to keep per slice per mipmap level data for a 2D texture array or cubemap. But it doesn't make sense for 2D or 3D textures.
I went through 2 iterations that I didn't deem worthy of showing to the team and creating a PR.
Now with the plan for a v2 version #6666 the breaking of the API doesn't seem so daunting.
I'm working on a third iteration of refactoring the API but this time trying to separate each type of texture in it's own class:

  • Texture2D
  • Texture2DArray
  • TextureCube
  • Texture3D
    with Texture remaining the base class (preventing it from being instantiated)
    I think this make more sense , allows for cleaner API for each type of texture and allows for lighter texture objects in general, (e.g Texture2D objects won't come with all the overhead of maintaining a cubemap, 2d texture array or 3d texture).

If this is something that the team won't consider please let me know so I can stop going into this direction, otherwise I'll continue towards a PR.

@mvaligursky
Copy link
Contributor

Thanks for working on this @heretique.

I find it unlikely to go ahead with the split of the Texture class into multiple classes, as that would have large impact to our user base / Editor.

Maybe we can just extend the _levels:

  • for 2D textures, it would not be a single data per mipmap, but an array to support 2D texture array (so if you have 3 textures, each _levels would be an array of 3
  • cubemap would have an array of 6
  • cubemap array would have an array of numCubemaps * 6

I have not looked in detail to see if this is possible.

We could also add a new member, for example _data which would follow this layout. And change _levels to setter/getter to convert from the current format if needed.

@heretique
Copy link
Contributor

Thanks for the feedback @mvaligursky I'm going to take a new stab at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue feature request
Projects
None yet
Development

No branches or pull requests

6 participants