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

Preview image distorted when zoom to 33% or 50% using a lens correction profile in combination with 'Auto fill' #1791

Closed
Beep6581 opened this issue Aug 11, 2015 · 30 comments
Assignees
Labels
patch provided type: enhancement Something could be better than it currently is

Comments

@Beep6581
Copy link
Owner

Originally reported on Google Code with ID 1807

When using a lens correction profile, the preview image at 50% zoom is broken. With
some Canon profiles, it not zoom to 50%, but to maximum depth. Other profiles (like
the Pentax for the Amsterdam photo sample) produces a extremly distortion at 50%.

You can download a sample image with the lense profile at http://www.terminal.io/tmp/4623/example.zip

Steps to reproduce:

- Open the photo
- zoom to 50%
- load the lens correction profile

Reported by cytrinox on 2013-03-26 20:29:29

@Beep6581 Beep6581 self-assigned this Aug 11, 2015
@Beep6581
Copy link
Owner Author

Confirmed in Gentoo about two weeks ago. I noticed no errors in the saved image, but
the preview was broken. Will try to reproduce and bisect tomorrow.

Oduis hope you don't mind if I assign you.

Reported by entertheyoni on 2013-03-26 22:16:48

  • Status changed: Accepted
  • Labels added: Priority-Medium, Usability, OpSys-All
  • Labels removed: Priority-Low

@Beep6581
Copy link
Owner Author

Sorry, as you know, since I switched to LR I don't do real patches any more, just builds.
However if any dev is seriously interested in taking over the LCP engine, I'd be happy
to have a Skype call to introduce him how the code works.

Reported by oduis@hotmail.com on 2013-03-29 10:51:01

@Beep6581
Copy link
Owner Author

Aww pity for us :]

From the forum:
by gs » Fri Mar 29, 2013 2:27 pm
(...)
I have the same problem when I apply the LCP profile to .nrw files from my P7100.
No problem instead with .NEF files.
RT4.0.10.36 on Win7_32
Saluti
Giorgio

Reported by entertheyoni on 2013-03-30 00:08:34

@Beep6581
Copy link
Owner Author

@2:

@odius: Ich schick' dir eine mail, sobald ich aus dem Urlaub zurück bin.

@english native speakers: I'll contact Odius, as soon as I'm back.

Reported by heckflosse@i-weyrich.de on 2013-03-30 00:20:50

@Beep6581
Copy link
Owner Author

Morgen Ingo,

Sehr schön! oduis at oliverduis.  Meine Skype-Name ist "oliverduis", dann können wir
den Code parallel via Bildschirmfreigabe anschauen.

Schönen Urlaub
Olli

Reported by oduis@hotmail.com on 2013-03-30 08:34:24

@Beep6581
Copy link
Owner Author

Had a first look. Broken preview is not caused by LCP, but by using 'Auto Fill', at
least with the sample file. But nevertheless, there also seems something wrong, when
not using 'Auto Fill'. Is is clearly visible, when viewing the upper left corner of
the sample picture at different zoom levels. 100% preview is not corrected at all.

Reported by heckflosse@i-weyrich.de on 2013-04-09 17:19:21

@Beep6581
Copy link
Owner Author

Reported by heckflosse@i-weyrich.de on 2013-04-09 17:19:52

@Beep6581
Copy link
Owner Author

Just for information. ImProcFunctions::getTransformAutoFill converges wrong direction
with sample picture and 50% preview, which results in about 5000% magnification of
the preview. At zoom-levels lower than 50% it converges correct, independent of the
zoom-level, to the same number every time. Will have a look ... but need some time!

Reported by heckflosse@i-weyrich.de on 2013-04-09 21:49:59

@Beep6581
Copy link
Owner Author

Reported by heckflosse@i-weyrich.de on 2013-04-09 21:50:17

  • Status changed: Started

@Beep6581
Copy link
Owner Author

From RT-Manual: 'Auto filling is also not supported'. Maybe we should disable 'Auto
Fill' when using LCP...

Reported by heckflosse@i-weyrich.de on 2013-04-10 13:09:58

@Beep6581
Copy link
Owner Author

Didn't find a RT changeset, where LCP in combination with 'Auto Fill' works with this
picture at 50% preview, hence I change priority to 'low'.

Reported by heckflosse@i-weyrich.de on 2013-04-10 22:20:04

  • Labels added: Priority-Low
  • Labels removed: Priority-Medium

@Beep6581
Copy link
Owner Author

Worked at this one today. Think I found the bug. Will post a patch today.

Ingo

Reported by heckflosse@i-weyrich.de on 2013-12-06 20:02:40

@Beep6581
Copy link
Owner Author

Have some problem when crop touches the border. Maybe I can post a patch tomorrow...

Reported by heckflosse@i-weyrich.de on 2013-12-06 23:20:22

@Beep6581
Copy link
Owner Author

'Autofill' in combination with LCP also leads to wrong results when zooming to 33% and
moving for example to upper right corner. The safe way is to do the same as for 100%
preview also for zoom >= 33%, which is to not calculate the 'auto fill' in this cases.
That doesn't have any influence on the final output. The only difference is, that the
change in preview which was before at 100% now is at 33%.

That's what the attached patch does.

Ingo

Reported by heckflosse@i-weyrich.de on 2013-12-11 15:19:49

  • Status changed: PatchSubmitted

- _Attachment: [issue1807_00.patch](https://storage.googleapis.com/google-code-attachments/rawtherapee/issue-1807/comment-14/issue1807_00.patch)_

@Beep6581
Copy link
Owner Author

Before: http://i.imgur.com/2rm6AEH.jpg
After : http://i.imgur.com/Dp6GDWH.jpg
Before: http://i.imgur.com/4Qu6Z8j.jpg
After : http://i.imgur.com/HIwAK1f.jpg
Before: http://i.imgur.com/0tRpeK6.jpg
After : http://i.imgur.com/IJTUQOr.jpg
Before: Clicked + once, to take me to 50%, instead I see this: http://i.imgur.com/hklj6g3.png
After : http://i.imgur.com/09rnvr9.jpg
Before: http://i.imgur.com/TPLiu4s.jpg
After : http://i.imgur.com/OeUsCe0.jpg

Before: Distortion Correction works up to and including 33%, malfunction at 50%, no
effect at 100%.
After: Distortion Correction works up to and including 25%, no effect at 33% or more.
In both before and after patch, Navigator preview and thumbnail changes correctly regardless
of main preview zoom level.

Reported by entertheyoni on 2013-12-12 21:33:31

@Beep6581
Copy link
Owner Author

If you try with a smaller preview you also get wrong output (magnified) when looking
at the top right corner of the example image at 33%. That's the reason why I disabled
it from this level upwards.

Ingo

Reported by heckflosse@i-weyrich.de on 2013-12-12 22:01:15

@Beep6581
Copy link
Owner Author

So it's correct that we cannot see LCP distortion correction at >=33%?

Reported by entertheyoni on 2013-12-12 22:08:46

@Beep6581
Copy link
Owner Author

Yes. Even before patch the preview was not consistent for different zoom levels (have
a look at the upper right corner at different zoom levels with a smaller preview area)!.
We have to decide whether we want a incorrect corrected preview from some level upwards
or a not corrected preview...

Reported by heckflosse@i-weyrich.de on 2013-12-12 22:29:53

@Beep6581
Copy link
Owner Author

I can try to show the LCP distortion at all zoom levels and only disable the 'Auto Fill'
from 33% upwards. That would be a better solution, I think.

Reported by heckflosse@i-weyrich.de on 2013-12-12 22:33:12

@Beep6581
Copy link
Owner Author

My problem with LCP correction is that the correction is carried out on a low resolution
image to begin with, so once it's carried out the image looks blurred and does not
reflect the real result. This is a significant problem for me. If we disabled LCP preview
at all zoom levels, that would solve my issue. However if someone uses an LCP profile
which makes a big difference, they will be screwed.

The real solution would be to calculate a *sharp* preview representative of the saved
image at every zoom level. (Unrealistically?) big job. Your idea of turning off just
Auto Fill sounds good. For my problem, a checkbox "Preview" to let me use LCP distortion
correction but to prevent it from screwing up the sharpness of the preview would suffice.
Or I could just enable LCP as the last thing I do before saving...

Reported by entertheyoni on 2013-12-12 22:50:39

@Beep6581
Copy link
Owner Author

It's not LCP which causes the problem, but the combination of LCP and 'Auto Fill'!

Perhaps the best way is to disable 'Auto Fill' when 'LCP' is used. This would be also
conform to the RT Manual, as I stated in #10 half a year ago. But in this case, as
I'm not a GUI-programmer, another dev should do that.

Ingo

Reported by heckflosse@i-weyrich.de on 2013-12-12 23:28:52

@Beep6581
Copy link
Owner Author

Issue 1473 has been merged into this issue.

Reported by entertheyoni on 2014-04-01 13:04:49

@Beep6581
Copy link
Owner Author

Issue 2544 has been merged into this issue.

Reported by entertheyoni on 2014-11-10 09:37:17

@Beep6581
Copy link
Owner Author

Beep6581 commented Sep 4, 2015

Options:
1- Don't show the LCP's effect in preview at all,
2- Keep the buggy behavior as is,
3- Automatically disable Auto-fill from 33% upwards (either just from the preview or completely)
4- Automatically disable Auto-fill when enabling LCP
5- Fix the code so LCP works well with Auto-fill at all zoom levels.

1, 2 and 3 are unacceptable and from the discussion it seems 5 is not to be expected. This leaves us with 4. Any objections?

@Beep6581
Copy link
Owner Author

Beep6581 commented May 3, 2016

Let's implement point 4 - Automatically disable Auto-fill when enabling LCP. The fact of Auto-fill being disabled should be reflected in history so that the user gets some feedback that this was done.

Starting point:

diff --git a/rtgui/lensprofile.cc b/rtgui/lensprofile.cc
index a7a6344..0362dc1 100644
--- a/rtgui/lensprofile.cc
+++ b/rtgui/lensprofile.cc
@@ -167,6 +167,7 @@ void LensProfilePanel::onUseDistChanged()
     useDistChanged = true;

     if (listener) {
+        listener->panelChanged (EvTransAutoFill, M("GENERAL_DISABLED"));
         listener->panelChanged (EvLCPUseDist, ckbUseDist->get_active() ? M("GENERAL_ENABLED") : M("GENERAL_DISABLED"));
     }
 }

It sends a message to the history panel, but doesn't actually disable Auto-fill yet.

@Beep6581
Copy link
Owner Author

Beep6581 commented May 30, 2016

The commit above, in branch lcpautofill, disables Auto-Fill when loading an LCP file in non-batch mode and when enabling LCP Distortion Correction in non-batch mode. In both cases you get notified about this via the History panel.

It does not disable Auto-Fill when you load a PP3 which has both enabled, because the idea is that by the time you create such a PP3 you are well aware of the issue and if you have both enabled then you did that knowingly.

@Beep6581
Copy link
Owner Author

Pull request opened:
#3318

Floessie added a commit that referenced this issue Jun 3, 2016
Automatically disable Auto-Fill when enabling LCP Disortion Correction, #1791
@Floessie
Copy link
Collaborator

Floessie commented Jun 3, 2016

Fixed by @Beep6581 and me in 8b5cf38.

@Beep6581
Copy link
Owner Author

Beep6581 commented Jun 6, 2016

There is a bug where if you close a photo with Auto-Fill enabled, regardless whether an LCP is used or not, and you restart RawTherapee (crucial, cannot reproduce this without restarting RT), then the photo will load with Auto-Fill disabled if you re-open it in the Editor tab.

@Beep6581
Copy link
Owner Author

Beep6581 commented Jun 6, 2016

Ah there already is an issue for the bug.

@Beep6581 Beep6581 closed this as completed Jun 6, 2016
agriggio added a commit that referenced this issue Apr 12, 2017
This was a workaround for an old bug (#1791 and #3765) that is no longer necessary
@agriggio agriggio mentioned this issue Apr 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch provided type: enhancement Something could be better than it currently is
Projects
None yet
Development

No branches or pull requests

2 participants