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

Match Nuke's default for CheckerBoard's centerlineWidth #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Shrinks99
Copy link
Contributor

Natron's default is currently 1px, Nuke's is 3px. Seeing as all the other values match Nuke it would make sense for this one to match it as well?

Natron's default is currently 1px, Nuke's is 3px.  Seeing as all the other values match Nuke it would make sense for this one to match it as well?
@Shrinks99 Shrinks99 changed the title Match Nuke's default for centerlineWidth Match Nuke's default for CheckerBoard's centerlineWidth Jun 5, 2021
@devernay
Copy link
Member

devernay commented Jun 7, 2021

Unfortunately, changing the default requires more boilerplate:

  • create a new major version of the plugin
  • make sure both the version with default value of 1 and with the default value of 3 are available.
    The reason is that values that don't change from the default are not written to the project file. If you just change the default, projects that were designed using the previous version will give different renders.

I know this is just the checkerboard plugin, but these are the rules we've been applyin to all plugins since 2013.

I think it's ok to be slightly different from Nuke, so I'm not in favor of merging this.

@Shrinks99
Copy link
Contributor Author

Shrinks99 commented Jun 7, 2021

If those are the rules then that's how it is and I won't complain too much ;) I do personally like the 3px value as I find it easier to see in higher resolution renders but it's not really worth too much hassle.

One default value I actually do have a problem with is the Blur border conditions. By default they're set to "Black" instead of "Nearest" which produces the vignette artifact that I'm sure you're aware of. I'm really at a loss as to when this would be preferable but maybe you've got an answer and I'm missing something?

Default (Black):
blackborder

Nearest:
nearestborder

Nuke (and Photoshop I think) notably also match the nearest option and don't actually have the setting to begin with. I've seen other applications do the same thing.

Perhaps also changes for the next major version? I think compatibility with other applications defaults is good as long as those defaults make sense. I don't see either of these changes really breaking anything for people (resulting in negative consequences, they obviously will change renders) but go ahead and close this, or just keep it open and don't merge until a later date when that eventually rolls around? I'm pretty sure old versions of the plugins are always available both packaged with old versions of Natron and in the releases tab here for other hosts, I think that's everything people should need to render old projects?

@devernay
Copy link
Member

devernay commented Jun 7, 2021

The nearest option in blur generates terrible artifacts when you blur roto shapes. maybe Nuke adds a 1-pix border to all roto shapes to fix that.
Nukes's blur default is bad for many reasons, and they had to add those "black outside" checkboxes almost everywhere because of that "nearest" default.
The correct way to blur an image is Blur with black border then unpremult. Blur with nearest will cause flickering when highly contrasted objects come near the image border, because one pixel virtually generate an infinite set of pixels.
Generate a simple animation with a white rectangle on black background that crosses the image from left to right and then disappears. you will clearly notice the flickering on Nuke when it disappears. Not on Natron

@Shrinks99
Copy link
Contributor Author

This makes sense... Thanks for the detailed walk through.

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