PixarAnimationStudios / OpenUSD

Universal Scene Description
http://www.openusd.org
Other
6.08k stars 1.21k forks source link

In Hydra/hdStorm, there's no API to force a texture to reload #1352

Closed jerry-huxtable-foundry closed 3 years ago

jerry-huxtable-foundry commented 4 years ago

Description of Issue

hdStorm has a mechanism to detect whether a texture has changed on disk and so needs reloading. If there's no disk file corresponding to the texture, there's no API to force a reload.

Steps to Reproduce

The background to this is that in our DCC, we are passing textures for the UsdUVTexture node directly to hdStorm without writing them to disk. In order to do this, we generate an asset path for the texture using our own file extension, say, "texture93.magictexture". We have an image file format plugin which claims to read ". magictexture" files, pretends to read the file and produces the texture data. This works really well.

The textures can change very frequently as users move sliders and the like so we'd like a way to force a texture to reload. If we were writing files to disk, this would happen automatically as hdStorm calls "stat" on the files to detect changes, but in our case there's no actual file on disk corresponding to the texture. We're reluctant to make one for performance reasons and the complications of managing the lifetime, cleaning up after crashes and so on. Currently, our workround is to generate a unique filename for every changed texture, but we suspect this is likely to have a bad effect on the internal texture cache as well as filling up memory with no-longer needed TfTokens and the like.

It would be nice to have an API to force a texture to reload. An HdDirtyBit we can set from the scene delegate would be ideal. Another option would perhaps be to allow the file format plugins to return a modification date and avoid the call to "stat".

System Information (OS, Hardware)

All

Package Versions

All

Build Flags

N/A

radon199 commented 4 years ago

Regardless of other ways of fixing this I think having a modification time for file formats would be a good thing overall. We have a similar virtual file format were the data it produces can be updated outside of USD. The data does have a consistent time stamp and that could be compared to see if the data needs to be reloaded or if USD can keep using what it has.

spiffmon commented 4 years ago

@radon199 , this should already be true for any client code that accesses data through an ArAsset (which I'm pretty sure the Storm texture system does), because you can only get an ArAsset through your resolver, which can give you GetModificationTimestamp() for the underlying asset.

radon199 commented 4 years ago

@spiffmon I see what we did. To get around the issue of having to have a file exist when using a file format we have been passing anon: as the identifier which allows us to bypass that, but in turn it means that the layer path is not passed through the resolver. Because if this the layer has not changed as far as Usd is concerned even though the data backed by the format may be different given the same arguments ( one of our arguments can be a concrete version number or a token that means “pull the latest version in the database” ). Calling Reload even with force does not cause the layer to reload, and so we end up doing the same thing as the above to force a reload, twiddle the identifier in a meaningless way. Now maybe that is all wrong from the Usd perspective but it’s work well enough for us so far.

jerry-huxtable-foundry commented 4 years ago

Just to follow up on this with regard to reloading textures: I investigated the ArAsset thing, but it turns out that Storm's texture system calls ArchGetModificationTime directly on the resolved asset and doesn't go via the ArResolver, so a custom ArResolver doesn't solve the problem.

Jerry

On Wed, Oct 7, 2020 at 7:47 PM Colin notifications@github.com wrote:

@spiffmon https://github.com/spiffmon I see what we did. To get around the issue of having to have a file exist when using a file format we have been passing anon: as the identifier which allows us to bypass that, but in turn it means that the layer path is not passed through the resolver. Because if this the layer has not changed as far as Usd is concerned even though the data backed by the format may be different given the same arguments ( one of our arguments can be a concrete version number or a token that means “pull the latest version in the database” ). Calling Reload even with force does not cause the layer to reload, and so we end up doing the same thing as the above to force a reload, twiddle the identifier in a meaningless way. Now maybe that is all wrong from the Usd perspective but it’s work well enough for us so far.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/USD/issues/1352#issuecomment-705126030, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOPBEGM2QN6RBPRFRTFGRTLSJSZVBANCNFSM4SEMIBAQ .

c64kernal commented 4 years ago

Hi @jerry-huxtable-foundry -- are you looking in GlfTextureRegistry? What version of Storm/USD distribution are you using? Depending on the version, GlfTextureRegistry represents the old texture system that's no longer being used by Storm. The new one (HdSt_TextureObjectRegistry) unfortunately doesn't provide any more help to you, but it currently doesn't do any file-modification-time-based invalidation. That's something that we will need to add still... which hopefully we can do around the same time we provide the API requested here...

jerry-huxtable-foundry commented 4 years ago

I'm looking at HdSt_TextureObjectRegistry. It's definitely calling ArchGetModificationTime, but I don't know if it's doing anything with that information. I'm on Apple's Metal branch of USD/Storm.

Jerry

On Fri, Oct 9, 2020 at 12:15 AM George ElKoura notifications@github.com wrote:

Hi @jerry-huxtable-foundry https://github.com/jerry-huxtable-foundry -- are you looking in GlfTextureRegistry? What version of Storm/USD distribution are you using? Depending on the version, GlfTextureRegistry represents the old texture system that's no longer being used by Storm. The new one (HdSt_TextureObjectRegistry) unfortunately doesn't provide any more help to you, but it currently doesn't do any file-modification-time-based invalidation. That's something that we will need to add still... which hopefully we can do around the same time we provide the API requested here...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PixarAnimationStudios/USD/issues/1352#issuecomment-705870873, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOPBEGMQQCXPA7KXNVNV7I3SJZBZXANCNFSM4SEMIBAQ .

jilliene commented 4 years ago

Filed as internal issue #USD-6405