Rawtherapee: Wrong crop in thumbnail view for vertical images

[agriggio]

There's a problem in displaying the crop area on thumbnails of vertical pictures. See screenshot. RAW from: https://pixls-discuss.s3.amazonaws.com/original/2X/d/d198acee5386038eebdf8e91084f2c9cf3fd3892.ORF

PP3 in attach (neutral with auto levels to make the picture a bit more visible, and crop)

Seems to happen only for vertical (i.e. with height > width) pictures. Pictures with "landscape" orientation work fine.

P1280191.ORF.pp3.txt

[Beep6581]

I've been experiencing the same issue but the result is even worse. I can't find an issue number, so I must have only discussed this on IRC.

Raw files and sidecars available from http://rawtherapee.com/shared/test_images/colortarget/

[TooWaBoo]

Confirmed. Same in Win 10 x64 Gtk3.22.9

[agriggio]

I'm taking a look at this

[agriggio]

Here's my patch. Can somebody else please test on Raw Images for which get_FujiWidth() returns non-zero, as I don't know which they are...? (also other testing is fine, of course)

diff --git a/rtengine/rtthumbnail.cc b/rtengine/rtthumbnail.cc
--- a/rtengine/rtthumbnail.cc
+++ b/rtengine/rtthumbnail.cc
@@ -550,9 +550,9 @@


     if (ri->get_FujiWidth() != 0) {
-        tpp->scale = (double) (height - ri->get_FujiWidth()) / sqrt(0.5) / h;
+        tpp->scale = (double) (height - ri->get_FujiWidth()) / sqrt(0.5) / (rotate_90 ? w : h);
     } else {
-        tpp->scale = (double) height / h;
+        tpp->scale = (double) height / (rotate_90 ? w : h);
     }

     // generate histogram for auto exposure

[heckflosse]

@agriggio I will test tomorrow with fuji image

[Floessie]

@agriggio Alberto, as you are a collaborator now, you can clone Beep6581/RawTherapee on your PC, create branches and push them back to Beep6581/RawTherapee directly. No need for agriggio/RawTherapee and alberto-dev anymore. :smile:

[agriggio]

@Floessie, thanks for the clarification. I did not want to push to dev before testing the Fuji case, and I thought creating a branch for this tiny patch was overkill. However, if that's the preferred/recommended workflow, I'll do that.

Regarding alberto-dev, it is not meant for you :-) It is the RT version that I use, typically merge of dev with the union of all the features I'm working on. So it will keep going, but you can safely ignore commit messages to it (I don't know if it is possible to configure github so that it doesn't notify of commits in that specific branch -- if you know if this is possible, please let me know!)

[Floessie]

However, if that's the preferred/recommended workflow, I'll do that.

No, it's absolutely fine the way you did it. I just wanted to give a hint.

I thought creating a branch for this tiny patch was overkill

Sometimes it starts with a little patch and grows during discussion. :wink:

but you can safely ignore commit messages to it

As long as the branch is not part of Beep6581/RawTherapee or a PR we don't get commit messages, as far as I can tell.

Best,
Flössie

[agriggio]

As long as the branch is not part of Beep6581/RawTherapee or a PR we don't get commit messages, as far as I can tell.

Ok, great. I asked because I see references to the commits in this page. But if you don't see them and/or they don't bother you, all the better! :-)

[Beep6581]

but you can safely ignore commit messages to it (I don't know if it is possible to configure github so that it doesn't notify of commits in that specific branch -- if you know if this is possible, please let me know!)

You could use a fork instead of the main clone.

[agriggio]

@Beep6581 you mean a fork of my fork? That seems a bit overkill...

[Beep6581]

@agriggio I didn't see that you forked already, so skip my comment.

[TooWaBoo]

It's not fixed.

[agriggio]

a bit more info? thanks!

[TooWaBoo]

Look at the thumbnail, it won't show the correct crop.
Isn't it fixed or did I missundstanding anything.

[agriggio]

Hi @TooWaBoo ,
yes, I looked at the crop. And I obviously see that there is something wrong. But I hope you might concede that I did some testing before closing it, so I'd say that it is equally obvious that it works here :smile:
Which means that there is some difference in our setup. This could include:

• platform,
• processing profile,
• and, most likely, the file format (jpeg or raw? if raw, produced by what camera?) we used.

So, my question about a bit more info could be expanded to: can you please tell me a bit more of your environment, and perhaps share the image and a pp3 if possible, so I can try to reproduce the problem here and debug it?

Thanks!

[TooWaBoo]

Here it comes: 😄
Nikon D90, Win 10 x64, File format: RAW
https://filebin.net/emh74gwwi9in17y2

[agriggio]

Thanks! I'll test immediately

[agriggio]

are you sure you are on the dev branch? Can you paste the contents of AboutThisBuild.txt?

[heckflosse]

I had the same yesterday. After clearing the cache it worked fine.

[TooWaBoo]

It's from my own fork. The dif to Beep-dev is only the "Reset to Default" feature.

I will do the test with Beep-dev again after it's compiled.

[agriggio]

@TooWaBoo ah sure, @heckflosse is right. You have to clear the cache, as you might still have the wrong version in there...

[TooWaBoo]

Cleared cache and now it's OK.