image_pipeline icon indicating copy to clipboard operation
image_pipeline copied to clipboard

Adds scaling option for calibrators

Open onurtore opened this issue 1 year ago • 5 comments

New personal computers can handle the input without the need for downscaling, this commit makes it parametric.

onurtore avatar Nov 18 '22 17:11 onurtore

@JWhitleyWork, Friendly ping.

onurtore avatar Dec 04 '22 11:12 onurtore

@onurtore Thanks for the PR. However, this pretty significantly changes the behavior of the node. I'm OK with the idea that you're presenting but not with the change in the default behavior. Instead, can you check for the existence of the parameter and, if it isn't defined, use the current behavior?

JWhitleyWork avatar Dec 04 '22 20:12 JWhitleyWork

Hi @JWhitleyWork , You are right, I shouldnt have changed the default behaviour, many people using this node.

While I fixed that with my latest commit, I belive the current version of the code is a little bit problematic, see the scale parameter is the scale of the input image relative to 640x480, however this behaviour does not look like intuitive, I believe when someone use such parameter it would expect making scale >1.0 scales up the input image, however the current implementation decreases it, so maybe I should change how the scale param calculated. Anyway, I belive this version is still acceptable if its okay, I can create a seperate PR for what I mentioned

onurtore avatar Dec 18 '22 13:12 onurtore

@JWhitleyWork, Friendly ping.

onurtore avatar Dec 30 '22 10:12 onurtore

@JWhitleyWork, Friendly ping.

onurtore avatar Jan 29 '23 10:01 onurtore