pulsejet / memories

Fast, modern and advanced photo management suite. Runs as a Nextcloud app.
https://memories.gallery
GNU Affero General Public License v3.0
3.2k stars 86 forks source link

unassigned face > selection problem #721

Closed EVOTk closed 1 year ago

EVOTk commented 1 year ago

Describe the bug I have a selection problem when I start to have selections, some photos are no longer selectable.

Also, when I assign a group of photos (e.g. 10), sometimes some photos are not assigned.)

To Reproduce Steps to reproduce the behavior:

Screenshots

https://github.com/pulsejet/memories/assets/45015615/8c572d86-bda4-4e6b-94a1-d0442a0ba3ce

https://discord.com/channels/1100157156742926406/1100158171621904567/1125686985063944193

Platform:

Additional context Add any other context about the problem here.

g3n35i5 commented 1 year ago

I'm having the same issue. I've tested it with Firefox and Chrome.

pulsejet commented 1 year ago

This is a confirmed bug; there's just no easy fix yet, unfortunately.

The workaround for now is to select the pictures separately.

EVOTk commented 1 year ago

Unfortunately, I don't have the skills to help you. I wish you good luck with this.

Thanks again for the work you do here!

g3n35i5 commented 1 year ago

I started to look into the problem and here are my findings so far.

The problem seems to be in the SelectionManager. When a photo is selected, the function selectPhoto is called. There it is checked if the selected photo already exists in the selection store. I have found the comment

https://github.com/pulsejet/memories/blob/abbed47d244a295bd3ae06f788dd4582e983a45f/src/components/SelectionManager.vue#L574-L576

and understand the problem as follows: The selection store is just a map between the photo ID and the photo, but does not contain the information about the faceid. Would it make sense to change the selection store from Map<number, IPhoto> to Map<[number, number], IPhoto>, where the first parameter of the key is still the photo ID and the second is the face ID?

pulsejet commented 1 year ago

@g3n35i5 your analysis is spot on. The only hurdles are that this needs you to know the key at all places, which I'm not sure is available right now, and this is a massive refactor.

g3n35i5 commented 1 year ago

Thanks @pulsejet for your answer. I'm totally new to nextcloud app development and I have very little experience with vue.js so far i would still like to try to tackle the problem.

I'm having the following questions:

1) I am wondering if the list of unassigned faces contains duplicate rectangles for the same picture. Can it happen that "recognize" classifies the same face multiple times? For example, if there are two people in one photo, can there be more than one detected face per person?

2) If not, would it be possible cluster the unassigned faces by their similarity index? If we do this, there could be a "select all faces in this category" button which assigns all selected faces to a person. The threshold for the similarity could be configurable with a slider in the UI.

pulsejet commented 1 year ago

@g3n35i5 if a face is "unassigned", all we have is a face vector that we could fetch from the backend. One person would have one detection in general unless one image has multiple images of the same person (e.g. a collage of photos).

Implementing a similarity finder might be possible, but that does need the UI to fiddle with recognize internals. This is

  1. Non trivial. We need to fetch the vector (overhead, lots of it), plus run the algorithm in the browser.
  2. Breakable, since recognize might change something without warning.

The most ideal way to do this would be inside recognize itself, probably using a search DAV command.

Edit: BTW, this has nothing to do with this issue. For this issue, all we need is to use the key instead of file id for the selection, with the caveat that the entire API of selection manager changes.

g3n35i5 commented 1 year ago

@pulsejet another idea: Would it be more easy to just use the index of the photo in the grid for the selection map?

# Pseudo Code
FOR index, photo in photos:
    <Photo ... index=index photo=photo />

<SelectionManger ...>

# In the selection manager

# Key changes from photo.fileid to photo index
selection = new Map<number, IPhoto>
pulsejet commented 1 year ago

@pulsejet another idea: Would it be more easy to just use the index of the photo in the grid for the selection map?

It's very hard to define an index, generally speaking. Everything gets lazy loaded, so it's hard to predict the position of any kind photo. Plus these positions might keep changing, e.g. if a photo is edited or deleted or new photos get added.

The ideal solution here is to just use the key (that's exactly what it's designed for: a globally unique identifier).

g3n35i5 commented 1 year ago

@pulsejet another idea: Would it be more easy to just use the index of the photo in the grid for the selection map?

It's very hard to define an index, generally speaking. Everything gets lazy loaded, so it's hard to predict the position of any kind photo. Plus these positions might keep changing, e.g. if a photo is edited or deleted or new photos get added.

The ideal solution here is to just use the key (that's exactly what it's designed for: a globally unique identifier).

So you mean that the key is a unique property on the IPhoto object? This would make the refactoring quite easy or am I wrong?

/**
   * Vue key unique to this object.
   * 1/ File ID by default.
   * 2/ Indexed if duplicates present.
   * 3/ Face ID for people views.
   */
  key?: string;

I'm trying to have a look at it the next days when I have time :+1:

EVOTk commented 1 year ago

It's perfect ! Thx