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

confusing phrasing for num_mip_levels #1241

Closed
bashbaug opened this issue Aug 29, 2024 · 5 comments · Fixed by #1255
Closed

confusing phrasing for num_mip_levels #1241

bashbaug opened this issue Aug 29, 2024 · 5 comments · Fixed by #1255

Comments

@bashbaug
Copy link
Contributor

The spec currently says:

num_mip_levels must be 0 unless the cl_khr_mipmap_image extension is supported, in which case it must be a value greater than 1 specifying the number of mipmap levels in the image.

I think this must should be a may instead, and it should probably should say "greater than or equal to 1" instead of "greater than 1"?

@bashbaug
Copy link
Contributor Author

I've reopened this issue to determine exactly what num_mip_levels means. See:
#1255 (comment)

@bashbaug
Copy link
Contributor Author

I made a quick tester over the weekend to see how this behaves on our implementation and in short it behaves consistent with the updates in #1255 (this is also the behavior I'd expect, intuitively). I tested and found:

  1. Creating an image with num_mip_levels equal to zero creates an image with a single mip level.
  2. Creating an image with num_mip_levels equal to one also creates an image with a single mip level.
  3. Creating an image with num_mip_levels equal to N creates an image with N mip levels.

So, I don't see any reason to change or revert #1255.

@karolherbst can you give this a try on your implementation?

https://github.com/bashbaug/SimpleOpenCLSamples/blob/mipmap-image/samples/images/10_mipmapimage/main.cpp

Here is the output on my system for the three scenarios above (-p2 picks the platform I'm running on, and -mN picks the value passed to num_mip_levels):

bashbaug@bashbaug-newpc:~/git/SimpleOpenCLSamples/install/RelWithDebInfo$ ./mipmapimage -p2 -m0
Running on platform: Intel(R) OpenCL Graphics
Running on device: Intel(R) Arc(TM) A750 Graphics
Device supports cl_khr_mipmap_image.
Creating a 32 x 32 image with 0 mip levels...
Initializing mipmap image...
Found 0 mip levels.
At base level: found 1.000000, wanted 1.000000
At mip level 0: found 1.000000, wanted 1.000000
At out-of-range mip level 1: found 1.000000
At out-of-range mip level 2: found 1.000000
At out-of-range mip level 3: found 1.000000
bashbaug@bashbaug-newpc:~/git/SimpleOpenCLSamples/install/RelWithDebInfo$ ./mipmapimage -p2 -m1
Running on platform: Intel(R) OpenCL Graphics
Running on device: Intel(R) Arc(TM) A750 Graphics
Device supports cl_khr_mipmap_image.
Creating a 32 x 32 image with 1 mip levels...
Initializing mipmap image...
Found 1 mip levels.
At base level: found 1.000000, wanted 1.000000
At mip level 0: found 1.000000, wanted 1.000000
At out-of-range mip level 1: found 1.000000
At out-of-range mip level 2: found 1.000000
At out-of-range mip level 3: found 1.000000
bashbaug@bashbaug-newpc:~/git/SimpleOpenCLSamples/install/RelWithDebInfo$ ./mipmapimage -p2 -m3
Running on platform: Intel(R) OpenCL Graphics
Running on device: Intel(R) Arc(TM) A750 Graphics
Device supports cl_khr_mipmap_image.
Creating a 32 x 32 image with 3 mip levels...
Initializing mipmap image...
Found 3 mip levels.
At base level: found 0.333333, wanted 0.333333
At mip level 0: found 0.333333, wanted 0.333333
At mip level 1: found 0.666667, wanted 0.666667
At mip level 2: found 1.000000, wanted 1.000000
At out-of-range mip level 3: found 1.000000
At out-of-range mip level 4: found 1.000000
At out-of-range mip level 5: found 1.000000

@karolherbst
Copy link
Contributor

yeah, that matches my interpretation as well. I think what mostly confused me is that the wording can be confusing depending on how you understand those terms.

If "number of mipmaps" means levels additional to the base level or levels in total. It's the latter and you seem to agree here. If that was your understanding all along then yes, nothing needs to be fixed.

However I think it makes sense to clarify this in the spec and specifically state that 0 and 1 are identical.

@bashbaug
Copy link
Contributor Author

Please review #1272, which adds additional clarification for "num_mip_levels".

@bashbaug
Copy link
Contributor Author

Closing after merging #1272.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants