0001259: use thumbnails of subsection instead of the whole image in person specific views

Notes
(0003081) bmcage (administrator) 2007-09-27 18:36	I'll have a look. As I'm writing the xml export, I have just thought of a problem. It looks in your code that if data is not set, that it is 0, like in the old code. I find that confusing (without having applied the patch). Would it not be better that lowerX is 100 and upperY is 100, so that changing nothing indicates the entire picture? Actually, most imaging software start in left top corner (upper) with (0,0) and have (100,100) in the other corner (negative Y axis), but I did not run the code yet, so I don't know how orientation is done here. Anyway, for the export to XML, it is no problem to export -1 for non-set data, that is, if non-set data actually exists, as I suppose the spinbox always returns data...

(0003082) tgehrig (reporter) 2007-09-27 18:54	In the old code I think there was sometimes (0,0,0,0) and sometimes None for not set subsections. I also find the naming lowerX/Y and upperX/Y really confusing. I just took it over from the spinbutton names. Because as you said before: what does lower x really mean? Or lower in any case? Is "lower" related to the position in the image or to the value of the spinbutton meaning a smaller value than that of upper x/y? Perhaps that should be renamed?! The behaviour is currently that I take the minimum of lower/upper x/y and the maximum of it to determine the boundaries, so upper and lower or left and right are not actually used by their namings. If they would be we would have to deal with negative widths and heights or restrict the values of the spinbuttons interactively. Nevertheless I find having None for rectangle still a good choice for not set subsection values, but the default values for the spinbutton should probably be (0,0,100,100) if rectangle==None, since that is what None actually does in that case. It outputs the whole thumbnail. I wouldn't export a not set subsection to xml, since that would be unnecessary redundancy. But if one of the rectangle values is set then the others should probably also be well defined, otherwise the subsection rectangle doesn't make any sense.

(0003083) bmcage (administrator) 2007-09-27 19:15	I posted a question on devel list about this. So you take origin in bottom left corner, normal axis. Fine with me. I'll rename the attributes to something better if nobody on the list comes up with something else. I'm not sure about taking the minimum/maximum without respect of what user gives, might confuse some users. I think you should leave lower and upper connected as point, not mix, and correct if you want different input in your routine. The math is not that hard to convert to positive width/height with correct corners. You could have a function taking the set values from database/GUI, and transforming it to correct lower and upper corner, positive width/height, before sending it to the thumbnail routine. Thanks for the work you already put into this!

(0003084) tgehrig (reporter) 2007-09-27 19:33	I'd prefer the origin at the top-left corner. I don't quite understand how taking min/max is different from what you propose!? Since the definition of lower/upper x/y is confusing anyways the user might not be much more confused by using the min/max;-), since the result is immediately displayed. Suppose that lower means smaller value and upper larger value, what would you expect the action to be if lower is larger than upper? The only thing I can think of is either doing it with min/max or restricting the range of the spinbuttons so that this will not happen, which could be annoying for the user. An alternative would be to switch from the concept of lower/upper x/y to upper-left (x, y), width and height.

(0003088) bmcage (administrator) 2007-09-28 03:38	Sorry, it was late, and I'm under pain killes due to fractured ribs. Ok, Brian on the list said top-left corner, so that it is. You are right about the min/max We do not want to upgrade old databases (would mean a lot larger path), so we are stuck with the present 4 fields meaning two corners, so no width/height. We do not want to restrict users to what they enter, so do no validation on the spin buttons, let them just enter stuff. Conclusion: we rename the fields to corner1_x, corner1_y and corner2_x corner2_y Your min max makes sure width and height are positive, and that's it. Agree? Should anyone implement mouse selection, the button down event would be corner1 and button up corner2. Do you make those changes, or are you fed up by now? I didn't look in your patch now, but I assume you consider a minimum width/heigth, so that width/height of zero is considered something else. Let's also think of a better name for subsection, or add a tooltip to upperx, ... labels explaining the origin is in top left.

(0003094) tgehrig (reporter) 2007-09-28 07:48	Well, the user is at least restricted to the range between 0 and 100 (that is at least how it was before), which I interpret as percentage of the original image (hope that is correct;-)). I still prefer x,y,w,h, but if that is not possible corner 1/2 x/y is okay. But you will still have the upgrade problem if users interpreted the values with an origin at the bottom left. But that is something that cannot automatically be resolved:-(. Or we add a radio buttom which tells which origin to use for the given values, but I think that messes up the gui unnecessarily. The width and height is always checked to be positive, so if it is 0 the whole image or none is displayed, depending in which view the thumbnail is displayed. I would like to implement the mouse selection, but I have no idea how to do the selection rectangle (I am not so much a gui programmer:-/). In any case if mouse selection is done, the subsection preview should probably be always the whole image and probably zoomable. Otherwise one cannot select a region with greatest accuracy of stepsize 1. Perhaps instead of subsection we could name it region of detail!? I don't really know how something like that is called in english;-).

(0003095) tgehrig (reporter) 2007-09-28 07:57 edited on: 2007-09-28 08:00	Another idea, how to make it more intuitiv what which of these four values could mean, one could arrange the 4 spinbuttons on a cross instead of just beside each other. It could look like: top y left x -----+------ right x bottom y But the problem with negative width/height would still be there. Or we use negative width/height to indicate flipping, meaning switching top and bottom or left and right. But this could escalate to something that gets near an image manipulation program;-).

(0003096) bmcage (administrator) 2007-09-28 08:03	tobias, just leave it in the database as it, with 2 corners. There is a 1-1 identification to point and width/height anyway. However, in the GUI you could use left-top corner, and width/heigth. I don't mind that, but for upgrade reasons, 2 corners in database so conversion on read and write. Your choice. About mouse selection of the region: add an eventbox under the picture. Then grab mouse click on the eventbox. The mouse down will return position in the eventbox. You can use mouse-drag and release to obtain where it was released. The coordinates of mouse down and mouse release can be calculated to 1-100 in respect to the dimensions of the eventbox. In eg person editor you find an eventbox under the image to show the popup on right-click, you can look at that code. Let's stick with the present patch for now however, and commit that, and if you do mouse selection, add a new issue to the tracker. We would need two pictures then, original, and selection, in my opinion.

(0003097) tgehrig (reporter) 2007-09-28 08:17 edited on: 2007-09-28 08:17	The thing with the eventbox was clear to me. I just wanted to display a selection rectangle while the selection takes place and after it. Is that done automatically? And is the range 0-100 to be with respect to the thumbnail frame or the picture itself. First of all the thumbnail frame is scaled by 97 as specified in const.py and not 100. And if you have for example in the frame an thumbnail that is only 50x100 should the x coordinates not match only the region of the image? Because off-image region doesn't make sence. Or did I misunderstand you?

(0003099) bmcage (administrator) 2007-09-28 08:39	Ok, Zsolt or Don would be good people to hold this discussion with, they might know the answer from the top of their head. I suggest you tell me if the patch for now is finished. Then write a mail to devel list with questions you ask here about the selection rectangle. Probably some fancy stuff is needed. Note that eg inkscape/gimp uses gtk, althoug not in python. Looking in the selection code there, should give you an idea how it is done with gtk to draw the rectangle. The power of OSS, you can look at how others do it, and learn from it. If you don't read C, and nobody has an idea on the devel list, I don't mind browsing to gimp code, but let's start with the easy thing first.

(0003100) tgehrig (reporter) 2007-09-28 09:14	Well, for now I would say the patch is finished. Sure it is not perfect, but it is working;-).

(0003102) shura (developer) 2007-09-28 14:05	I seem to be late to this, but here's my 2 cents. The four numbers were supposed to be the corners of a region, within the image, in pixels. The origin was thought to be in the lower-left corner of the image. The 0,0 - 100,100 range was just something to use until we get around to finishing it. It was not thought to be perfcent, always pixels. With origin at lower left and "lower" meaning "smaller number" in english, I think it made sense. Of course, one has to work out the cases of region wider than the image, negative offset, etc. Probably with UI. Also the idea was to draw a red frame with the region over the image, so that the user would immediately see the selection.

(0003103) shura (developer) 2007-09-28 14:12	The reasoning for pixels, I should add, is that they're always integer, so no division and related rounding is necessary. Of course, it adds the difficulty of changing spinbutton max to the size of a current image, but this should not be hard.

(0003104) bmcage (administrator) 2007-09-28 14:37	Ok, so as not to discourage Tobias, I commited this patch already. Now I will do some changes to this code as I noted here (corner, ...) I already saw one bug: the image in the gallery updates after save, but not the preview of the person. When I finish, I'll let you know. Tobias, could you then change percent to pixels? I agree to Alex that that is probably best. If rectangles is difficult to add to the code, we could add the following: left click in preview: set corner1, right click: set corner2 Not so nice as dragging a rectangle, but it does the job just as fine. Ok, back to coding.

(0003108) tgehrig (reporter) 2007-09-28 17:32	Initially, when I saw the subsection dialog I also thought it would be pixels and opened the image in an image editor to get the accurate pixel positions, but when I wanted to enter it, they were cut off at 100:-/. That is why I thought it would be percentage. But I also prefer the use of pixels for the region over percentage. But what should be the origin now, bottom-left or top-left? Since every image editor I know uses top-left, that is my favourite;-), as already mentioned. About the red rectangle that was the initial idea: Was that meant to be only in the preview for the selection? Or also in the thumbnails used everywhere else? Since I didn't yet look at the code of gimp, I still don't know how this is implemented;-). But I can surely write immediately the code to select the region (at least for now blindly;-)) in the whole image as thumbnail and put a rectangle on it after the mouse button up event is received and update the region thumbnail at the same time. Updating the region thumbnail while selecting the region would be something I would really like, but I fear that this will be getting really slow, at least if I do it like I do it now, which is probably not the most efficient way;-). But I think if mouse selection should be implemented the image should be larger in which the selection takes place, particularly when the selection should be in pixels and not in percentage. The question is, how much space is allowed;-)? And to get the most accuracy a zoom feature would be best.

(0003109) shura (developer) 2007-09-28 17:41	> But what should be the origin now, bottom-left or top-left? Since every image editor I know uses top-left, that is my favourite;-), as already mentioned. It is arbitrary. As long as the user is clear on what the controls are it should be fine. I relied in the past on "Lower corner" having "lower" (i.e. smaller) y coordinate, which should be intuitive for a layman, but this can be changed. Do as you want, and then the UI has to be adjusted to make the facts clear. > About the red rectangle that was the initial idea: Was that meant to be only in the preview for the selection? I only thought of this being the additional portion of info. E.g. you have a family picture. It is referenced from each family member, with a peson-specific region indicating that person on a group photo. I think re-draw the rectangle in response to the spinbutton values change is adequate. Could be called on the 'changed' signal of the spinbuton (or whatever this signal is caleld).

(0003112) bmcage (administrator) 2007-09-28 20:31	Ok, as brian says, use top left. I did my changes, all is now called corner1 or corner2, default is 0,0,100,100 not 0,0,0,0, provided upgrade path, and save of None if no valid data (important as not to have invalid data in xml on export), and correct update bug on Editperson. Tobias, if you want, you can not change code so it uses pixelwidth instead of percent. One note of caution, pixel has disadvantage that it can be large to work with (detailed scans eg), and means that if person replaces his image in the media by a better scanned image, all his references are now wrongly aligned. A percentage means that on replace of a image by better scan, the section remains the same. So I actually think working independent of pixel in the reference is not a bad thing. It also avoids things in database which depend on pixel sizes of images on directory. Still TODO however is get the lock icon in a better position, it looks ugly where it is now.

(0003175) bmcage (administrator) 2007-10-01 16:00	region export/import to xml now implemented in trunk Tobian, could you still do: 1/ update the preview picture if focus leaves the spin buttons. I see that if I enter 170 in the spin button, then it goes to 100, and the image is updated. The update should happen on focus leave, or you could add a 'refresh' button that redraws the preview 2/ place lock icon in a nicer position. It looks orphaned now 3/ the placement of the spin buttons can perhaps improve. They look a bit far apart. Not so important this

(0003184) tgehrig (reporter) 2007-10-02 10:04	I have uploaded a quick patch for the thumbnail refresh problem, when the values are entered via keyboard. For the lock icon I have uploaded a seperate patch, where I centered the icon about the whole width of the window instead about the 2 spinbutton columns. I also used the same column spacing as for the frame below with the description and so forth. Perhaps that is what you meant?!

(0003185) bmcage (administrator) 2007-10-02 11:18	Ok, I think all is done. Looks good. svn up Tobias, if you have time, you should check the support of the reports for these partial pictures (website, relationship, ...), and open new tickets for that.

Issue History
Date Modified	Username	Field	Change
2007-09-27 18:23	tgehrig	New Issue
2007-09-27 18:23	tgehrig	File Added: gramps-svn-r9013-subsection.patch.bz2
2007-09-27 18:36	bmcage	Note Added: 0003081
2007-09-27 18:54	tgehrig	Note Added: 0003082
2007-09-27 19:15	bmcage	Note Added: 0003083
2007-09-27 19:33	tgehrig	Note Added: 0003084
2007-09-28 03:38	bmcage	Note Added: 0003088
2007-09-28 07:48	tgehrig	Note Added: 0003094
2007-09-28 07:57	tgehrig	Note Added: 0003095
2007-09-28 07:57	tgehrig	Note Edited: 0003095
2007-09-28 07:59	tgehrig	Note Edited: 0003095
2007-09-28 08:00	tgehrig	Note Edited: 0003095
2007-09-28 08:03	bmcage	Note Added: 0003096
2007-09-28 08:17	tgehrig	Note Added: 0003097
2007-09-28 08:17	tgehrig	Note Edited: 0003097
2007-09-28 08:39	bmcage	Note Added: 0003099
2007-09-28 08:43	bmcage	Status	new => assigned
2007-09-28 08:43	bmcage	Assigned To	=> bmcage
2007-09-28 09:14	tgehrig	Note Added: 0003100
2007-09-28 14:05	shura	Note Added: 0003102
2007-09-28 14:12	shura	Note Added: 0003103
2007-09-28 14:37	bmcage	Note Added: 0003104
2007-09-28 17:32	tgehrig	Note Added: 0003108
2007-09-28 17:41	shura	Note Added: 0003109
2007-09-28 20:31	bmcage	Note Added: 0003112
2007-10-01 16:00	bmcage	Note Added: 0003175
2007-10-02 09:54	tgehrig	File Added: gramps-thumbnail_refresh-r9059.patch.bz2
2007-10-02 10:01	tgehrig	File Added: gramps-lock_icon-r9059.patch.bz2
2007-10-02 10:04	tgehrig	Note Added: 0003184
2007-10-02 11:18	bmcage	Status	assigned => resolved
2007-10-02 11:18	bmcage	Resolution	open => fixed
2007-10-02 11:18	bmcage	Note Added: 0003185
2009-06-27 03:13	romjerome	Relationship added	related to 0002513