mbaeuerle / Briss-2.0

Briss 2.0 is intended to be a GUI Update for the Briss PDF cropping tool.
GNU General Public License v3.0
499 stars 48 forks source link

fix: give precedence to older rectangles when selecting or deleting #43

Closed cleydyr closed 2 years ago

cleydyr commented 2 years ago

Fixes #41

mbaeuerle commented 2 years ago

I just can confirm that it works for these two cases:

I think there are several cases other cases to be adapted:

Maybe a more pragmatic approach would be to just add new rects at the beginning of the list. This would simplify things quite a bit and as there will be no more than a couple of rects per mergedPannel anyway the performance impact is negligible.

cleydyr commented 2 years ago

Hey, @mbaeuerle . I got your suggestion. What about the numbers printed in each crop? Is it okay to print 1 to the most recently drawn rectangle?

mbaeuerle commented 2 years ago

@cleydyr you're right didn't think of that. But I would just enumerate the crops in reverse (in update initialize cropCnt with crops.size() and decrease it in each iteration) to match the current behavior.

mbaeuerle commented 2 years ago

One thing I missed: I think the draw order is directly reverse to the selection order. The oldest rectangle should be drawn first, all others on top of it. But when checking for selection the newest one should be first. Therefore adding to my last comment I would not just to reverse the enumeration but the whole drawing part.

cleydyr commented 2 years ago

Hey, @mbaeuerle . So, let me try to summarise the idea.

  1. the newest crop should have number 1 as its label;
  2. if crop B is newer than crop A, then the label of B should be strictly lesser than the label of A;
  3. if crop A and B have an overlapping area AND B is newer than A, then B should have precedence when choosing a crop based on the location of a point inside that area.
  4. if crop B is newer than A, then A should be drawn first.

Does this get the gist of the changes?

mbaeuerle commented 2 years ago

Hi, @cleydyr Number 1 and 2 are not exactly what I intended. Newer rects should have higher numbers. This is also the current implementation on master. The first (or oldest) rect starts with number 1. But the two other points are right.

I think it's easier to state it in another way: From a user perspective everything stays the same except for the selection of the crops. To be specific point 3 you mentioned should be implemented:

if crop A and B have an overlapping area AND B is newer than A, then B should have precedence when choosing a crop based on the location of a point inside that area.

So for the implementation we discussed two different possibilities: