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

IC/TS: add optional force argument to invalidate() method #2133

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Jan 7, 2019

This works the same way as the force argument to invalidate_all() --
if true, invalidates unconditionally; if false, invalidates only if the
file's modifiation date has changes since the cache first opened it.

The default is true for back compatibility, but probably you will often
want false, only invalidating if the file has changed.

This works the same way as the `force` argument to `invalidate_all()` --
if true, invalidates unconditionally; if false, invalidates only if the
file's modifiation date has changes since the cache first opened it.

The default is true for back compatibility, but probably you will often
want `false`, only invalidating if the file has changed.
@lgritz lgritz merged commit df710e9 into AcademySoftwareFoundation:master Jan 14, 2019
@lgritz lgritz deleted the lg-invalidate branch January 14, 2019 18:37
@ThiagoIze
Copy link
Collaborator

ThiagoIze commented Feb 5, 2019

This is causing one of our tests to fail where we load a texture, flip the texture_automip setting, invalidate_all(false), and load the texture again. OIIO has code to check for this in invalidate_all:

            // Invalidate if any unmipped subimage didn't automip but
            // automip is now on, or did automip but automip is now off.
            if (sub.unmipped
                && ((m_automip && f->miplevels(s) <= 1)
                    || (!m_automip && f->miplevels(s) > 1))) {
                all_files.push_back(name);
                break;

but we then a few lines later do a invalidate(f, force); which in this case has force==false and because the file itself didn't change we don't invalidate. I think we want to instead do a invalidate(f, true) since we've already done the checks in this function and know we want an invalidation.

@lgritz
Copy link
Collaborator Author

lgritz commented Feb 5, 2019

Oh yes, you're quire right. Fix coming up.

lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Feb 5, 2019
When we recently made invalidate(file) have a 'force' parameter, we
botched it by not so forcing a particular call in invalidate_all().
In the line being changed here, we're dealing with a list of files
that definitely need invalidation due to special tests, so once
we've established that, we want to force it.

Pointed out by @ThiagoIze in a post-merge comment in AcademySoftwareFoundation#2133
lgritz added a commit that referenced this pull request Feb 5, 2019
When we recently made invalidate(file) have a 'force' parameter, we
botched it by not so forcing a particular call in invalidate_all().
In the line being changed here, we're dealing with a list of files
that definitely need invalidation due to special tests, so once
we've established that, we want to force it.

Pointed out by @ThiagoIze in a post-merge comment in #2133
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

Successfully merging this pull request may close these issues.

2 participants