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

Feature #8968: Actions are not applied to all grouped images when collapsed #1703

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@Skrapion
Copy link
Contributor

commented Aug 1, 2018

Implements feature #8968: Actions are not applied to all grouped images when collapsed

See discussion here: https://redmine.darktable.org/issues/8968

This patch does a few things:

  1. It allows you to click on a rating in a grouped image and apply that rating to the whole group.
  2. When selecting grouped images, it selects all the images in that group, making it easier to do mass exports, etc.
  3. It adds a new 'grouping' collection to filter group leaders or group followers. This allows you to access the old behaviour if you need it. For instance, if you want to do a mass export of just the representative images.
  4. Fixed some minor bugs as I found them. The biggest was that if the group leader was filtered out by collection rules, the whole group was filtered out.

@Skrapion Skrapion changed the title Feature #8968 Feature #8968: Actions are not applied to all grouped images when collapsed Aug 1, 2018

@TurboGit
Copy link
Member

left a comment

I did not test it yet, but this looks promising to me. I like the grouping handling as described.

Show resolved Hide resolved src/common/collection.h Outdated
Show resolved Hide resolved src/libs/collect.h Outdated
@TurboGit

This comment has been minimized.

Copy link
Member

commented Sep 11, 2018

I've read the corresponding Redmine issue and it seems that there is no consensus about this. We probably need more discussion before including this.

@Skrapion

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2018

@TurboGit I agree with your review. When I get a chance, I'll fix and test the changes.

As for consensus, @houz is the only person unconvinced that rating a group should affect all images in that group. I'm hoping that he thinks the new grouping collection I added offers the best of both worlds, but we won't know until he sees this PR.

I can say that I've been using this code for a while now, and it makes it much easier to review/reject RAW+JPG groups when I'm importing a film roll. I've also found it's nice that I can make an HDR group or a timelapse group, and then select all the images in that group with a single click and export the whole thing. And then when I actually make an HDR image out of a group (which might require exporting to Hugin) I can set the final image as the group header, and then set the collections to "group leaders" when I want only the final HDR images, or "group followers" when I want only the source images. It's great!

@Skrapion

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2018

@TurboGit I applied your suggested changes, and also updated to it merges cleanly.

@TurboGit

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

You seem to have integrated a rawspeed submodule change in the commits. Also, not at this stage that it would be nice to squash all the commits together (into a single commit).

@TurboGit TurboGit self-assigned this Oct 31, 2018

@TurboGit TurboGit added this to the 2.6 milestone Oct 31, 2018

@Skrapion Skrapion force-pushed the Skrapion:master branch 2 times, most recently from 05a9749 to 4c308f3 Oct 31, 2018

@Skrapion

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2018

Okay, tracked down the rawspeed external (not sure why that didn't merge into my branch cleanly) and squashed commits.

Thanks for your patience.

@TurboGit
Copy link
Member

left a comment

I've tested a bit, and I like this. I'll do some more test. A minor change.

Show resolved Hide resolved src/common/ratings.c Outdated
Show resolved Hide resolved src/common/ratings.c Outdated
Show resolved Hide resolved src/common/ratings.c Outdated
* Added collection filter to show only group leaders or group followers.
* Fixed a bug that caused all images in a group to be filtered out if their representative image was filtered out by the collections filter.

* Clicking a rating on a group now applies that rating to the whole group.

* Selecting a group now selects all items in the group, and accurately reports how many images there are when grouped images are hidden.

@Skrapion Skrapion force-pushed the Skrapion:master branch from 4c308f3 to ef28371 Nov 1, 2018

@Skrapion

This comment has been minimized.

Copy link
Contributor Author

commented Nov 1, 2018

Good change. Glad you're enjoying the patch. Cheers!

@TurboGit

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Merged, thanks. Note that there were still some bool in your patch that I have fixed.

@TurboGit TurboGit closed this Nov 1, 2018

@Skrapion

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

Awesome, thanks!

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.