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

RendererServices::good() no longer used when generating textures #1122

Open
DeclanRussell opened this issue Mar 2, 2020 · 5 comments
Open

Comments

@DeclanRussell
Copy link
Contributor

Problem

While doing some texture work I noticed that the RendererServices::good() function is no longer being called when generating textures. The function serves as an interface to validate texture handles retrieved from RendererServices::get_texture_handle(). This was removed as of a5d8381 and the texture handle is now just checked if it is non NULL. What are the plans for this API moving forward? If it is no longer going to be used there should probably be some indication that it is deprecated.

Versions

  • OSL branch/version: master - 1.11.0
  • OS: All
  • C++ compiler: N/A
  • LLVM version: N/A
  • OIIO version: N/A
@fpsunflower
Copy link
Contributor

You are right. I am not sure what the original intent of the call was, but I don't think this API is really needed.

I can't think of a case where we need to validate the texture handle on the OSL side before trying to use it. The lookups themselves are supposed to signal the error when attempting to use a broken handle. Calling good() might tell you something is wrong, but it can't tell you what, so its not really useful for error reporting.

I think we should just deprecate/remove it.

@ee33
Copy link

ee33 commented Mar 2, 2020

FWIW, demand-loaded GPU textures might want something like that. For when the texture file hasn't been opened, so size etc are unknown. An Optix 7 demand-loading library is coming soon.

@fpsunflower
Copy link
Contributor

In this case, I would still expect get_texture_handle to do the work of preparing the GPU representation. If the texture is invalid, you still need to generate some representation that allows the error to be reported later on.

I don't see a need for OSL itself to need to know if the handle is "good" or not. Only the renderer (which implements these methods) should care, so the extra virtual entry point seems redundant.

@lgritz
Copy link
Collaborator

lgritz commented Mar 2, 2020

It was there for a reason. I need to look at the older code to remember why. Was it needed during optimization?

@fpsunflower
Copy link
Contributor

The spots where it was called before didn't really make much sense. It was only resetting the handle to null when it was not good(), even though a get_texture_handle call must, by definition always give you back the same thing.

If get_texture_handle() gives you back non-null, we assume its ok to use that at runtime, even if it happens to be a broken handle. It is up to the specifics of the gettextureinfo or texture calls to report the error to the user.

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

No branches or pull requests

4 participants