Fix mipmapLevel/RenderScale bugs related to overlay code.

[acolwell]

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:

• I've followed the contributing guidelines to the best of my understanding
• I've read and understood the contributing guidelines
• I've formatted my code according to Natron's code style
• I've searched the pull requests tracker to ensure that this PR is not a duplicate

PR Description

What type of PR is this? (Check one of the boxes below)

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (non-breaking change which does not add functionality nor fixes a bug but improves Natron in some way)
• Breaking change (fix or feature that would cause existing functionality to change)
• My change requires a change to the documentation
  • I have updated the documentation accordingly

What does this pull request do?

• Fixed a conversion from mipMapLevel to RenderScale that appeared to be the inverse of what it should have been. No other mipMapLevel to RenderScale conversion anywhere else in the codebase matched this particular conversion. Code that calls this function also tries to reverse the conversion expecting the more popular mapping which also implies that the conversion is a bug.

• Changed ViewerGL::mouseDoubleClickEvent() to use getCurrentRenderScale() to get the current mipMapLevel so that it matches all the other code that computes a RenderScale for a notifyOverlaysXXX method.

Have you tested your changes (if applicable)? If so, how?

I tried to find places where I could test these changes, but it appears that these 2 behaviors are not actually observed by any of the Natron plugins or nodes that they would affect.

The first fix is observable in changedParam() callbacks in plugins like HSVTool, Grade, Noop, ColorLookup, etc. but none of those plugins actually use the RenderScale value. The RenderScale gets passed to the plugin's processor and then the value is completely ignored during processing. I've verified the value passed to these plugins is correct now, but this doesn't appear to actually affect their behavior because they were not using the values to begin with.

The second fix is also observable, but existing code does not use the values. OFX Plugins don't receive the overlay double click event so there is no impact there. The RotoPaint and Tracking code ignore the render scale parameter for double click events so there is no impact to that code right now either.

Futher details of this pull request

I came across these issues while exploring some RenderScale/mipMapLevel refactoring ideas. Even though these issues aren't causing problems in the existing code, it seems worth fixing them. They could cause problems if the RenderScale values ever did get used and they are inconsistent with similar code.

[devernay]

nice catch! what about also renaming getCurrentRenderScale to getCurrentMipmapLevel , to avoid confusion? (btw, Mipmap or mipmap is wrongly capitalized MipMap or mipMap at many places), including this one)

[acolwell]

nice catch! what about also renaming getCurrentRenderScale to getCurrentMipmapLevel , to avoid confusion? (btw, Mipmap or mipmap is wrongly capitalized MipMap or mipMap at many places), including this one)

Thanks. Yeah. I was planning on doing that function renaming and some related duplicate code cleanup in a follow up. I was just trying to resolve the inconsistencies first. I was kind of wondering about the mipmap vs mipMap capitalization. I've seen both everywhere and wasn't sure which was preferred.

[devernay]

according to wikipedia, it's either mipmap or MIP map (MIP=multum in parvo), so that mipMap and MipMap are both wrong. I think most textbooks just say mipmap now

[acolwell]

according to wikipedia, it's either mipmap or MIP map (MIP=multum in parvo), so that mipMap and MipMap are both wrong. I think most textbooks just say mipmap now

Yeah I've always seen it all lowercase so I was surprised when I saw the camelCase. I was just trying to blend in. I'm happy to fix this across the codebase in a follow up. mipMapLevel -> mipmapLevel renderMappedMipMapLevel -> renderMappedMipmapLevel etc.

[devernay]

👍