Fix UI not updating when thumbtable image is rejected

[sayoder]

Fixes the following bug:
Steps to reproduce:
- Enter darkroom view
- Scale bottom panel UI small enough that rating overlay does not display on
thumbnail mouseover
- Hover over an image, and press R.
- Observe that the image does not appear to be rejected until we
enter another view where the rating overlay is displayed.

Previously, _image_get_infos short-circuited when a thumbnail did not have an overlay. When using keyboard shortcuts, there are situations where we still need to call _image_get_infos to appropriately update the UI.

[TurboGit]

Scale bottom panel UI small enough that rating overlay does not display on
thumbnail mouseover

What do you mean by this? A screenshot would help I suppose. TIA.

[sayoder]

@TurboGit - Here is a video where I demonstrate the bug.

This video shows four tests:

• Hover over non-rejected image in lighttable, press r.
  • Expect: thumbnail greyed out. This works.
• Hover over rejected image in lighttable, press r.
  • Expect: thumbnail no longer greyed out. This works.
• Hover over non-rejected image in darkroom thumbnail strip, press r.
  • Expect: thumbnail greyed out.
  • Actual: thumbnail remains solid until we switch back to lighttable view -- observe that it is greyed out there, and it retains that state when we switch to darktable view again.
• Hover over rejected image in darkroom thumbnail strip, press r.
  • Expect: thumbnail no longer greyed out.
  • Actual: thumbnail remains greyed out until we switch back to lighttable view, etc. etc. etc.

[sayoder]

@TurboGit , sorry I didn't answer your question directly. Here is a video that shows the behavior I'm talking about in that sentence. I think this behavior is intended, but I'm not sure.

When the thumbtable strip is too small, the rating overlay does not display when I mouse over a thumbnail.

[TurboGit]

I see, I could reproduce and thinks that the PR is ok. I'll let @AlicVB validate this part. TIA.

[AlicVB]

Thanks for the PR.
You've spotted the code problem, but I think we should be able to get a change with less impact on perfs (even if current proposal is not horrible)
Ping me if you need help !

[sayoder]

Hi @AlicVB , let me know if you think the newest commit addresses your concerns. The new logic is:

• gather the bare minimum info required to check if the image is rejected.
• if reject status has changed, update the UI class.
• if overlay is DT_THUMBNAIL_OVERLAYS_NONE, release cache and exit.
• continue gathering other info for other overlays

Thanks in advance!

[TurboGit]

@AlicVB : We are approaching the release, can you do a quick review and let me know if we can merge this in 4.8 or postpone for 5.0? TIA.

[TurboGit]

@AlicVB : When you have some free cycle for a review :) TIA.

[AlicVB]

First, I'm really sorry for the extra long delay. I've completely forgotten that PR...

Your proposal is far better, but I have a more "global" interrogation here

[AlicVB]

Is there any reason to close the if(img) condition here (and reopen it after) ?
I think it would even break the dt_image_cache_release just after (in case of OVERLAYS_NONE)

In fact I even think that current code in master is not fully right : in the case we fail to get the image cache, it actually "reset" the thumb->rating to 0. I would have said that in this case, we have to let everything untouched...
So for our case that would means :

• removing the thumb->rating = 0 line
• let all your change inside the if(img) condition
• add another check after that condition to exit if no overlay

What would you think of suvh a change ? @TurboGit : I would also like your opinion as I fail to trigger a case where we don't get the image cache...

Thanks !

[TurboGit]

removing the thumb->rating = 0 line

This needs to be properly tested to ensure there is no side effects. We maybe depends on the rating being 0 to report that an image is not found for example.

let all your change inside the if(img) condition

Right, seems better indeed.

add another check after that condition to exit if no overlay

Ok with that too.