Fix floating point exception while using `-H`

[boretom]

What kind of change does this PR introduce? (check at least one)

• [x] Bugfix
• [ ] Feature
• [ ] Code style update
• [ ] Refactor
• [ ] Build-related changes
• [ ] Other, please describe:

Does this PR introduce a breaking change? (check one)

• [ ] Yes
• [x] No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• [ ] When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• [ ] All tests are passing
• [ ] New/updated tests are included

If adding a new feature, the PR's description includes:

• [ ] A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Fix the floating point exception, it was simple enough. Before, it tried to grow the image and I guess img_resize is not made for that. wh in sh_image.c ended up 0 since the scale ratio got well below 1.

[boretom]

@posva hold on with this PR, not sure why I missed that -> although the exception is fixed now the height is only 1/2 of what is should be if height < img height.

[boretom]

I close this PR, I'm confused on how precision influenced the hight, if you got any insight on that @posva just let me know.

[posva]

I haven't been able to check the exact line where the program crashes but we don't do any upscaling with images, we only make them smaller (that's how it currently works with the width). If the provided height is larger than the image, it should be constrained to the actual height.

Then there is the resolution parameter that, by default use vertical half blocks to be able to display more pixels, that's why you can see some * 2 or / 2 in some places.

[boretom]

Thanks @posva, I got that yes.

Since I don't want to change any existing behaviour you may can explain to me the following:

If you run catimg -r 1 -w 0 test-images/mewtwo-front.png on the 2.6.0 master the resulting image will have width of 128 (double the width of the image which is 64x64). Run it with catimg -r 2 -w 0 test-images/mewtwo-front.png it's 64 characters width. I would have expected the ... -r 1 ... call to also output an image with 64 characters width.

Is that an intended behaviour?

I of course can replicate that behaviour but want to make sure it is how it should behave.

[boretom]

Ok I read your answer again so do I understand correctly that precision only really (internally) has an effect on the height then?

[posva]

that's intended behavior. The width and height should refer to the size in pixels. In order to make pixels (squares) with the space character, we need two of them, that's why we have twice the amount of characters horizontally but the regular amount vertically. When using -r 2, we use unicode to display 2 pixels inside of one characters, dividing by two the resulting height but keeping the horizontal amount of characters.

Let me know if that makes sense! I also haven't ran the program through gdb but the problem doesn't reproduce with all pictures. It does with this one:

./build/bin/catimg -H 500 photo.jpg

gives

[1]    2573 floating point exception  ./build/bin/catimg -H 500

[boretom]

Ok, I see. But does it make sense that a 64x64 pixel image run with -r 1 -w 0 ends up being 128 characters wide?

I did find the root cause using lldb and it was a rather dumb brain freeze of mine. But to really fix it I try to understand the height and width behaviour better.

the diff right now which I try to verify:

--- a/src/catimg.c
+++ b/src/catimg.c
@@ -156,8 +156,8 @@ int main(int argc, char *argv[])
     } else if (cols > 0 && cols < img.width) {
         scale_cols = cols / (float)img.width;
         img_resize(&img, scale_cols, scale_cols);
-     } else if (rows > 0 && rows < img.height) {
-        scale_rows = (float)(rows * 2) / (float)img.height;
+     } else if (rows > 0 && (rows * 2 / precision) < img.height) {
+        scale_rows = (float)(rows * 2 / precision) / (float)img.height;
         img_resize(&img, scale_rows, scale_rows);
     }

[posva]

Ok, I see. But does it make sense that a 64x64 pixel image run with -r 1 -w 0 ends up being 128 characters wide?

Yes! Because we need 2 characters to create one pixel and both w and H should refer to the size in pixels, not in characters, adapting to the resolution

[posva]

Go ahead with the PR! I thought there was also some floating casting not done properly to the different variables. Maybe using 2.f would highlight better compilation errors (I haven't done C in a long time, so I'm a little bit rusty 😄 )

[boretom]

On the danger of you loosing your patient:

Yes! Because we need 2 characters to create one pixel and both w and H should refer to the size in pixels, not in characters, adapting to the resolution

Ok, that means using mewtwo-front.png as an example:

• catimg -r 1 -H 64 ... you end of with a 64 rows x 128 columns image
• catimg -r 2 -H 64 ... you end of with a 32 rows x 64 columns image

... since resolution == 1 -> 1 pixel (height) == 1 character and resolution == 2 -> 2 pixel (height) == 1 character.

And catimg -r 1 -H 64 ... would still output a 64 rows x 128 columns image since the source image is a 64x64 pixel.

[posva]

That is correct!

On the danger of you loosing your patient:

lol, no, no worries