Add generate mipmap sample #497

greggman · 2025-03-21T19:12:39Z

We've got hello triangle. Generating mipmaps seems like a pretty common feature. Maybe a sample for it is good?

beaufortfrancois

Here's my drive-by review! Thanks @greggman for documenting those!

sample/generateMipmap/main.ts

beaufortfrancois · 2025-03-24T09:29:08Z

sample/generateMipmap/main.ts

+
+  canvas.toBlob((blob) => {
+    const img = new Image();
+    img.src = URL.createObjectURL(blob);


Shall we call revokeObjectURL when img is loaded?

why? It just clutters the sample.

img.decode().then(() => URL.revokeObjectURL(img.src))

it also makes the image not viewable if you right click and pick "Open Image in New Tab". Similarly if you click the link in DevTools.

It's best practice to revokeObjectURL when image is not needed anymore. The "Shall" meant it was a question ;)

beaufortfrancois · 2025-03-24T09:29:42Z

sample/generateMipmap/main.ts

+  });
+}
+
+const settings = { expand: false };


FWIW the word "expand" was confusing to me until I tried it.

changed to view 512x512. not sure what else to change it to dat.gui defaults to only around 12 characters.

sample/generateMipmap/main.ts

SenorBlanco · 2025-03-24T15:22:18Z

sample/generateMipmap/generateMipmap.ts

+export function generateMips(
+  device: GPUDevice,
+  texture: GPUTexture,
+  textureBindingViewDimension?: GPUTextureViewDimension


It looks like you never actually call this function with a value for this parameter, and always rely on the default. Is that correct? If so, it seems like you'll never exercise generating mips for cube maps or 1-layer arrays. Is that intentional or did I miss something?

Maybe I'm Compat-biased, but I feel like we should just make this parameter mandatory, rather than duplicate the browser's default logic if it's omitted.

That's the opposite of your advice when I wrote tests in the past. You've said in the past, only set textureBindingViewDimension if it's required, otherwise rely on the defaults.

I think someone coming into this just wants it to work without having the specify anything. It will work except for cube and 1 layer 2d-array. No one is likely to make a 1-layer 2d array so that leaves cube. For cube they'll get a validation error alone the lines of tried to use a 2d-array as a cube, can't do that in compat mode

Yes, I agree with myself. :) Don't pass tbvd if you don't need to. It's just that it seems a bit sad to have to duplicate the browser's default logic for it, but perhaps that's unavoidable if you want to have the simple case be simple (2D).

What I'm getting at is, the sample doesn't show how to correctly generate cube maps, even though the utility function suggests it can do it. So I feel like the utility function should either be stripped down to just 2d, or the sample should exercise cube maps as well.

All that said, I think this is a great sample and it's also fine as is.

I updated to show 4 cubes. One uses 2d, one 2d-array, one cube, one cube-array if it can, else cube

SenorBlanco · 2025-03-24T15:29:23Z

sample/generateMipmap/generateMipmap.ts

+
+/**
+ * Get the default viewDimension
+ * Note: It's only a guess. The user needs to tell us to be


For my own edification: this duplicates the guess logic from the browser, no? In which case it's not needed for the actual binding, just to determine the correct shader to use. In fact, I don't think this is actually passed to the textureBindingViewDimension in the TextureDescriptor. So this sample only works for the default dimensions, and wouldn't work if you actually set it to cube or 1-layer array? (This is fine I suppose I'm just trying to understand.)

The sample works for cube maps. The reason it doesn't pass the textureBindingViewDimension from the GPUTextureDescriptor is because the descriptor is unavailable. generateMips only input is the GPUTexture. The textureBindingViewDimesion is lost after texture creation. It's common to call generateMips after updating the contents of the first mip and it would be inconvenient if your code that wasn't keeping the texture descriptors around suddenly had to keep them around just for this function. As it is you don't have to keep the descriptor around. You just pass 'cube' in the case of cubemaps, otherwise it just works.

OK, so what you're saying is, this is a useful utility function and the given sample just exercises some of its capabilities. That's fine, I'm just trying to understand it.

I could add examples of all 4 types of textures 2d, 2d-array, cube, cube-array (skipped in compat). It would make the sample larger. Then the question of how to show them. I guess I could spinning draw cubes with and without mipmaps

If textureBindingViewDimension were reflected from the GPUTexture, could we get rid of this helper?
We already reflect other stuff, it seems like we can reflect this too, especially if it's useful.

In Core we would probably return undefined.

kainino0x

No additional comments from me!

We've got hello triangle. Generating mipmaps seems like a pretty common feature. Maybe a sample for it is good?

SenorBlanco · 2025-03-25T19:28:27Z

Hmmm.. on Chrome Linux Stable (134) with --enable-unsafe-webgpu (enabling Compatibility Mode), I get:

Uncaptured error:
Dimension (TextureViewDimension::Cube) of [TextureView of Texture (unlabeled 256x256 px, 6 layer, TextureFormat::RGBA8Unorm)] must match textureBindingViewDimension (TextureViewDimension::e2DArray) of [Texture (unlabeled 256x256 px, 6 layer, TextureFormat::RGBA8Unorm)] in compatibility mode.

While validating entries[2] as a Sampled Texture.
Expected entry layout: {sampleType: TextureSampleType::Float, viewDimension: 4, multisampled: 0}
While validating [BindGroupDescriptor] against [BindGroupLayout (unlabeled)]
While calling [Device].CreateBindGroup([BindGroupDescriptor]).

greggman · 2025-03-25T20:54:40Z

Sorry, it should be fixed now

greggman force-pushed the generate-mipmap branch from 35cd5f2 to a44e413 Compare March 21, 2025 19:17

greggman requested a review from kainino0x March 24, 2025 05:49

beaufortfrancois reviewed Mar 24, 2025

View reviewed changes

SenorBlanco reviewed Mar 24, 2025

View reviewed changes

greggman force-pushed the generate-mipmap branch from a44e413 to 9d9c5f3 Compare March 24, 2025 16:26

kainino0x approved these changes Mar 24, 2025

View reviewed changes

greggman force-pushed the generate-mipmap branch 2 times, most recently from ed36d1f to 9ce7a54 Compare March 25, 2025 03:32

Add generate mipmap sample

4e27bc2

We've got hello triangle. Generating mipmaps seems like a pretty common feature. Maybe a sample for it is good?

greggman force-pushed the generate-mipmap branch from 9ce7a54 to 4e27bc2 Compare March 25, 2025 03:45

greggman enabled auto-merge (squash) March 25, 2025 03:47

greggman merged commit fe8a2ce into webgpu:main Mar 25, 2025
1 check passed

Add generate mipmap sample #497

Add generate mipmap sample #497

Uh oh!

Conversation

greggman commented Mar 21, 2025

Uh oh!

beaufortfrancois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kainino0x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SenorBlanco commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greggman commented Mar 25, 2025

Uh oh!

Uh oh!

SenorBlanco commented Mar 25, 2025 •

edited

Loading