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

Fixing file preview functionality on import file dialog box #1588

Merged
merged 2 commits into from Dec 22, 2017

Conversation

Projects
None yet
4 participants
@peterbud
Copy link
Contributor

commented Dec 18, 2017

Calling gdk_pixbuf_loader_close is needed before gdk_pixbuf_loader_get_pixbuf.

Also restoring file preview functionality on the import dialog box, even when no preview is available in the image file - previously this functionality was commented out. Please check that I got the logic right.

Fixes bug #11866

Calling gdk_pixbuf_loader_close before gdk_pixbuf_loader_get_pixbuf
Also restoring file preview functionality on the import dialog box, even when no preview is available in theimage file

@peterbud peterbud changed the title Fixing file preview functinality on import file dialog box Fixing file preview functionality on import file dialog box Dec 18, 2017

// loader. We must do this before calling gdk_pixbuf_loader_get_pixbuf.
GError *error = NULL;
gdk_pixbuf_loader_close(loader, &error);
if(error)

This comment has been minimized.

Copy link
@houz

houz Dec 19, 2017

Member

No need to pass error and then free it. Just use NULL as the 2nd argument to gdk_pixbuf_loader_close() and check its return value.

@@ -694,8 +703,9 @@ static void _lib_import_update_preview(GtkFileChooser *file_chooser, gpointer da
g_object_unref(loader); // This should clean up tmp as well
}
}
if(have_preview && !no_preview_fallback)
if(!have_preview && !no_preview_fallback)

This comment has been minimized.

Copy link
@houz

houz Dec 19, 2017

Member

Nope, that is the wrong logic. The rotation has to be done when getting the thumbnail. You disabled it for when the first attempt worked out.
Calling gdk_pixbuf_new_from_file_at_size() has to happen iff the previous block failed, and the rotation logic has to be independent from that. It has to be determined if the rotation logic needs to be used when opening the whole file, too.

This comment has been minimized.

Copy link
@peterbud

peterbud Dec 19, 2017

Author Contributor

OK, now I get it.
Still not clear though the case of DNG files: if we try to open a DNG file, no_preview_fallback will be TRUE.

  • is it OK to read the DNG file with gdk_pixbuf_new_from_file_at_size() and only the thumbnail reading should be prevented, or
  • both the thumbnail reading and gdk_pixbuf_new_from_file_at_size() call should be blocked for those files?

This comment has been minimized.

Copy link
@houz

houz Dec 19, 2017

Member

Looking at the git log it seems that passing a DNG to gdk_pixbuf_new_from_file_at_size() is error prone: a56251c

This comment has been minimized.

Copy link
@peterbud

peterbud Dec 19, 2017

Author Contributor

OK, the latest commit does not call gdk_pixbuf_new_from_file_at_size() for DNG files.
As this comment is from 5 years ago, it might be that since then this has been fixed in libtiff(?) - but I cannot test it as I don't have hdr DNG file at hand.

}
if(no_preview_fallback || !have_preview)
if(!have_preview && no_preview_fallback)

This comment has been minimized.

Copy link
@houz

houz Dec 19, 2017

Member

When not having a preview (have_preview == FALSE) we have to go into this, no matter what no_preview_fallback says.

@TurboGit TurboGit added the bug label Dec 19, 2017

@houz houz merged commit 18fd9ff into darktable-org:master Dec 22, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@LebedevRI LebedevRI added this to the 2.4 milestone Dec 22, 2017

@peterbud peterbud deleted the peterbud:bugfix-11866 branch Dec 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.