FLTK logo

STR #2231

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]

Return to Bugs & Features | Roadmap 2.0 | Post Text | Post File | SVN ⇄ GIT ]

STR #2231

Application:FLTK Library
Status:5 - New
Priority:3 - Moderate, e.g. unable to compile the software
Scope:3 - Applies to all machines and operating systems
Summary:Browser doesn't recalculate column width when item has an image
Created By:isaque
Assigned To:Unassigned
Fix Version:Unassigned
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:

Post File ]
Name/Time/Date Filename/Size  
#1 isaque
21:25 Jul 30, 2009
#2 isaque
19:49 Aug 19, 2009

Trouble Report Comments:

Post Text ]
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(...);

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?

#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 ]


Comments are owned by the poster. All other content is copyright 1998-2024 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.