Page MenuHome
Differential D6456

Use 2D texture arrays for UDIM drawing in Eevee to avoid texture limit
ClosedPublic
Actions

Authored by Lukas Stockner (lukasstockner97) on Dec 20 2019, 2:34 AM.

Details

Reviewers
Clément Foucault (fclem)
Maniphest Tasks
T73133: I can only use 4 UDIM textures
Commits
rB7d8a18633540: Fix T73133: UDIM texture count in Eevee is limited by OpenGL
Summary

Based on @Clément Foucault (fclem)'s suggestion in D6421, this patch implements support for
storing all tiles of a UDIM texture in a single 2D array texture on the GPU.

Initially my plan was to have one array texture per unique size, but managing
the different textures and keeping everything consistent ended up being way
too complex.

Therefore, this patch uses a simpler version that allocates a texture that
is large enough to fit the largest tile and then packs all tiles into as many
layers as necessary.

As a result, each UDIM texture only binds two textures (one for the actual
images, one for metadata) regardless of how many tiles are used.

Note that this removes per-tile GPUTextures completely, meaning that it
breaks D6421.

Diff Detail

Repository
rB Blender
Branch
tilepack (branched from master)
Build Status
Buildable 6089
Build 6089: arc lint + arc unit

Event Timeline

Lukas Stockner (lukasstockner97) created this revision.Dec 20 2019, 2:34 AM
Harbormaster completed remote builds in B6083: Diff 20430.Dec 20 2019, 2:34 AM
Clément Foucault (fclem) added inline comments.Dec 20 2019, 2:45 AM
source/blender/gpu/shaders/material/gpu_shader_material_tex_image.glsl
373

code style: don't use camel case for local variables. Use snake case.

Clément Foucault (fclem) added inline comments.Dec 20 2019, 2:51 AM
source/blender/gpu/shaders/material/gpu_shader_material_tex_image.glsl
374

Use one load. Use RGBA32F texture for the metadata and predivide by arraysize.

Lukas Stockner (lukasstockner97) mentioned this in T72390: Improve UDIM functionality.Dec 20 2019, 9:58 PM
Lukas Stockner (lukasstockner97) updated this revision to Diff 20442.Dec 20 2019, 10:20 PM

Adressed review comments.

Harbormaster completed remote builds in B6089: Diff 20442.Dec 20 2019, 10:20 PM
Lukas Stockner (lukasstockner97) marked 2 inline comments as done.Dec 20 2019, 10:21 PM
Lukas Stockner (lukasstockner97) updated this revision to Diff 20443.Dec 20 2019, 10:49 PM

And here's workbench engine support, way easier than I expected.

Harbormaster completed remote builds in B6090: Diff 20443.Dec 20 2019, 10:49 PM
Clément Foucault (fclem) added inline comments.Dec 21 2019, 2:22 AM
source/blender/draw/engines/workbench/shaders/workbench_prepass_frag.glsl
6 ↗(On Diff #20443)

Red Flag here: Every samplers must have a valid texture (of their type) bound and assigned. If not the implementation crashes or could trigger some undefined behavior in some cases. We use quite a bit of them all over the place already. Maybe it would be nice to centralize it somewhere in GPU_texture.c.

Nevertheless, you need to create and bind dummy textures to these sampler when not used. Same for the sampler2D image sampler.

Clément Foucault (fclem) requested changes to this revision.Dec 21 2019, 2:27 AM
Clément Foucault (fclem) added inline comments.
source/blender/draw/engines/workbench/shaders/workbench_common_lib.glsl
143 ↗(On Diff #20443)

One thing I don't really like about sampler1DArray is that the layer dimension may be limited to 256.

However, the dimension that represents the number of array levels is limited by GL_MAX_ARRAY_TEXTURE_LAYERS, which is generally much smaller. OpenGL 4.5 requires that the limit be at least 2048, while OpenGL 3.0 requires it be at least 256.

Maybe you can use sampler2D here. I don't really see why you need sampler1DArray anyway.

This revision now requires changes to proceed.Dec 21 2019, 2:27 AM
Lukas Stockner (lukasstockner97) updated this revision to Diff 20509.Dec 26 2019, 9:23 PM
Lukas Stockner (lukasstockner97) marked an inline comment as done.

Good point about the unbound textures.

The solution I went with for now is to have a compile-time define and
build separate shaders for tiled and non-tiled. This should also
prevent any performance regression.

Harbormaster completed remote builds in B6113: Diff 20509.Dec 26 2019, 9:23 PM
Lukas Stockner (lukasstockner97) added inline comments.Dec 26 2019, 9:24 PM
source/blender/draw/engines/workbench/shaders/workbench_common_lib.glsl
143 ↗(On Diff #20443)

The layer dimension is hardcoded to 2, the number of tiles only affects width.

As for sampler1DArray - the main advantage is that I can keep the existing GPU_texture_from_blender, GPU_NODE_LINK_IMAGE_BLENDER etc. If both the regular non-tiled texture as well as the tile metadata have the same type, we'll need a lot more boilerplate code.

Clément Foucault (fclem) accepted this revision.Jan 13 2020, 2:45 PM

I approve this patch as my comments are mostly nitpicking.

source/blender/draw/engines/workbench/shaders/workbench_common_lib.glsl
150 ↗(On Diff #20509)

style: use 10.0 for float values. Some compilers will throw errors because of implicit conversion.

156 ↗(On Diff #20509)

Same here

source/blender/gpu/intern/gpu_draw.c
339

Style: Nitpicking here but I prefer to have GLenum data_type = main_ibuf->rect_float ? GL_FLOAT : GL_RGBA; than having a "big" if statement. Same holds for the following glTexSubImage3D.

source/blender/nodes/shader/nodes/node_shader_tex_image.c
135

A little strange but OK :)

This revision is now accepted and ready to land.Jan 13 2020, 2:45 PM
Jeroen Bakker (jbakker) mentioned this in D6421: Workbench Engine: Add support for UDIM textures.Jan 14 2020, 3:57 PM
Philipp Oeser (lichtwerk) mentioned this in T73133: I can only use 4 UDIM textures.Jan 15 2020, 5:25 PM
Philipp Oeser (lichtwerk) added a task: T73133: I can only use 4 UDIM textures.
Philipp Oeser (lichtwerk) added a subscriber: Philipp Oeser (lichtwerk).Jan 15 2020, 5:28 PM

@Lukas Stockner (lukasstockner97) : when committing this, you can close T73133: I can only use 4 UDIM textures

Closed by commit rB7d8a18633540: Fix T73133: UDIM texture count in Eevee is limited by OpenGL (authored by Lukas Stockner (lukasstockner97)). · Explain WhyJan 16 2020, 2:13 AM
This revision was automatically updated to reflect the committed changes.
Lukas Stockner (lukasstockner97) added a commit: rB7d8a18633540: Fix T73133: UDIM texture count in Eevee is limited by OpenGL.

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
36 lines
blenlib/
10 lines
intern/
98 lines
6 lines
blenloader/
intern/
32 lines
draw/
intern/
20 lines
gpu/
12 lines
intern/
5 lines
26 lines
467 lines
12 lines
shaders/
material/
95 lines
makesdna/
16 lines
4 lines
makesrna/
intern/
3 lines
12 lines
nodes/
shader/
nodes/
8 lines
38 lines

Diff 20442

View Options

source/blender/blenkernel/intern/image.c

Loading...
View Options

source/blender/blenlib/BLI_boxpack_2d.h

Loading...
View Options

source/blender/blenlib/intern/boxpack_2d.c

Loading...
View Options

source/blender/blenlib/intern/math_vector_inline.c

Loading...
View Options

source/blender/blenloader/intern/readfile.c

Loading...
View Options

source/blender/draw/intern/draw_manager_data.c

Loading...
View Options

source/blender/gpu/GPU_material.h

Loading...
View Options

source/blender/gpu/intern/gpu_codegen.h

Loading...
View Options

source/blender/gpu/intern/gpu_codegen.c

Loading...
View Options

source/blender/gpu/intern/gpu_draw.c

Loading...
View Options

source/blender/gpu/intern/gpu_texture.c

Loading...
View Options

source/blender/gpu/shaders/material/gpu_shader_material_tex_image.glsl

Loading...
View Options

source/blender/makesdna/DNA_image_types.h

Loading...
View Options

source/blender/makesdna/DNA_movieclip_types.h

Loading...
View Options

source/blender/makesrna/intern/rna_image.c

Loading...
View Options

source/blender/makesrna/intern/rna_image_api.c

Loading...
View Options

source/blender/nodes/shader/nodes/node_shader_tex_environment.c

Loading...
View Options

source/blender/nodes/shader/nodes/node_shader_tex_image.c

Loading...