[ Return to Bugs & Features | Roadmap 2.0 | Post Text | Post File ]

STR #2231

Trouble Report Files:

	Name/Time/Date	Filename/Size
	#1  isaque 21:25 Jul 30, 2009	column_shift.diff 2k
	#2  isaque 19:49 Aug 19, 2009	column_shift_v2.diff 2k

Trouble Report Comments:

	Name/Time/Date	Text
	#1  isaque 21:25 Jul 30, 2009	Whenever you have a multi column browser and you add an image to an item, like this: Browser b(...); Widget* item=b.add(...); item->image(an_image); The image shifts the whole browser line and make them skewed. I've created a patch that fixes that.
	#2  isaque 12:39 Aug 03, 2009	Please use test/browser.cxx to test it.
	#3  isaque 12:34 Aug 18, 2009	This patch seems to be working fine for quite some time. Anyone can apply it to the code? Thanks.
	#4  greg.ercolano 15:53 Aug 18, 2009	Hmm, I'm just looking over this patch, and am wondering about the last bit of code: + +  // Restore column width +  if (cols) cols[0] = saved_colw; It /seems/ like the conditions that that initialize 'saved_colw' are possibly different than the condition that restores the value. ie. saved_colw is effectively set on the conditions of:     if ( img && !(flags & ALIGN_CENTER) && (flags & ALIGN_LEFT) && cols) ..but saved_colw is saved back into cols[0] without any of those conditions needing to be met, meaning cols[0] might end up as zero. Is that correct?
	#5  greg.ercolano 16:01 Aug 18, 2009	Or in other words: ---- snip void Widget::draw_label(const Rectangle& ir, Flags flags) const {     int saved_colw = 0;  // VALUE INIT TO ZERO     ..     if (img) {         ..         if (flags & ALIGN_CENTER) {            ..         } else {            ..            if (flags & ALIGN_LEFT && cols) {                saved_colw = cols[0];    // ACTUAL VALUE SAVED HERE ONLY                ..            }         }     }     ..     if (cols) cols[0] = saved_colw;  // RESTORE USES DIFFERENT LOGIC ---- snip
	#6  isaque 08:33 Aug 19, 2009	Yeah, it's correct. I'm fixing that and I'll provide a new patch. Thanks for catching that.
	#7  isaque 19:49 Aug 19, 2009	Fixed the issue pointed by Greg - thanks again man!
	#8  AlbrechtS 04:23 Aug 20, 2009	Honestly said: I don't think that the provided patch is acceptable. You change Widget::draw_label() to achieve a better layout for a special case in the browser widget. What else may this affect? It's not enough to verify that it solves _your_ problem... Also, I don't understand these lines:       // Shift first column width, so labels after 1st column are lined up correctly.       if (flags & ALIGN_LEFT && cols) {             cols[0] -= ir.w();             if (cols[0]==0) cols[0]--;       } First point: what, if ir.w() > cols[] ? Are negative cols values acceptable? Second point: I can see why there is the special case "if (cols[0]==0)", but why do you decrement (and not e.g. increment) the cols value, resulting in -1 (again a negative value). That said, I'm not a FLTK 2 developer, I only read the code, and I may be wrong with this...
	#9  AlbrechtS 04:26 Aug 20, 2009	Correction: "First point" should read: "what, if ir.w() > cols[0] ? ..."
	#10  isaque 06:58 Aug 20, 2009	I used the same code that's used by Browser code to draw the plus sign and lines in the a treeview browser. The idea for cols[0]-- is to make it negative and make the column resizable. The code was used only when the image is on the left of the browser line because it was where the problem happens. I had to make the change on Widget_draw.cxx because it's there where the problem is. This code is responsible for all label draw routine, including the image draw.
	#11  isaque 07:02 Aug 20, 2009	The way this patch was coded, it only affects this case where it's a multicolumn label (browser case) with an image on the left. No other place was impacted. The issue pointed by Greg was fixed and I can't see any other impact besides that, so if you find any other issue, please let me know and I'll try to fix that as well.
	#12  AlbrechtS 07:50 Aug 20, 2009	What about this? After your patch, in line #246, there is:   if (!img) return; // don't call pop_clip if push_clip was not called And in line #252:   if (cols) cols[0] = saved_colw;
	#13  AlbrechtS 07:59 Aug 20, 2009	Oh, sorry, I didn't see that cols[0] is modified only if img is true, thus my last point is moot. But I still don't know if your code can't have side effects on other widgets (label drawing), where your special alignment is *not* wanted. Maybe some FLTK 2 developers should check that.

[ Return to Bugs & Features | Post Text | Post File ]