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

Update image infos in dr mode when switching using keyboard #56

Closed

Conversation

fgrollier
Copy link
Contributor

When navigating between images in darkroom mode using keyboard shortcuts (space/backspace) the "image information" panel isn't updated until the mouse is moved.

This patch fixes this by raising the "mouse moved" signal when the image changes.

Although it works exactly as expected, I'm really unsure this is the best to do the trick. At least it works.

(Patch related to #8827 and #8953).

@fgrollier
Copy link
Contributor Author

Mmm, maybe I don't understand you correctly, by I don't think this will be a solution.

My understanding of the bug is that we need a way to tell darktable that the image below the cursor as changed without the mouse actually moving. And as far as I can tell the proper way to do this in dt is to use DT_CTL_SET_GLOBAL(lib_image_mouse_over_id, ...) — because the metadatas updating code reads what's in this global variable to update itself — and it's this macro that triggers the mouse_moved signal as part of its job.

So, apart from duplicating the functionality of DT_CTL_SET_GLOBAL while stripping its "raise signal" part, I can't find another solution right now. But maybe I'm not intimate enough with darktable's code to handle this properly :)

@hanatos
Copy link
Member

hanatos commented Sep 26, 2012

looks good to me. the macro sends the signal and locks a mutex (which is not necessary because you're all in the gui thread, but also doesn't hurt).

@hanatos
Copy link
Member

hanatos commented Sep 26, 2012

actually the only critique i have is i probably would have put it into the select_this_image(imgid) function. any reason why not to do that? it seems to be called a couple more times.

@fgrollier
Copy link
Contributor Author

Well, if I understand correctly the code both solutions of modifying select_this_image or dt_dev_change_image should work the same way (the first is called by the later). The only time select_this_image is called without dt_dev_change_image is at darkroom's initialization time, but it's not of interest here. So I think there's no "real" difference in the case we're involved here.

That said, select_this_image looks more like a database oriented function, while dt_dev_change_image is more general, so I chose the second one. As I said before I'm not yet familiar enough with dt's internals, so these remarks may be irrelevant.

@hean01
Copy link
Member

hean01 commented Sep 30, 2012

This is fixed upstream using signals.

@hean01 hean01 closed this Sep 30, 2012
@LebedevRI LebedevRI modified the milestone: 1.1.x Oct 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants