gansm / finalcut

A text-based widget toolkit
https://github.com/gansm/finalcut/wiki/First-steps#first-steps-with-the-final-cut-widget-toolkit
GNU Lesser General Public License v3.0
1k stars 52 forks source link

FLabel Text Wrapping, Not Obeying Max Size Hint #43

Closed terranpro closed 4 years ago

terranpro commented 4 years ago

Hello, was experimenting with finalcut (already a huge fan, absolutely brilliant work) and found setMaximumWidth , setMaximumSize, etc. don't see to be obeyed when updating a label's text dynamically. Here's a minimal reproduction case for my scenario below.

The output looks something like the screenshot attached, with dots wrapping line after line. If you uncomment the commented two lines, I noticed it would enforce the minimum size as the maximum, and only 5 dots were displayed. Also to note, if I use FLabel::setGeometry or setSize, the maximum size is enforced, and no wrapping occurs.

Perhaps I am not using FLabel the way it was intended, but as a newcomer it seems like this is probably not intended and maximum hints should be enforced. Or maybe I made a simple mistake?

Thanks!

labelbugreport

#include <iostream>

#include <final/final.h>

namespace fc = finalcut;

class HelloDialog : public fc::FDialog
{
    fc::FLabel labelDim{this};

public:
    HelloDialog(fc::FWidget& widget)
        : FDialog(&widget)
    {
        setText("Hello Finalcut - Size Issue");
        setPos(fc::FPoint(10, 1));
        setSize(fc::FSize(50,25));

        //labelDim.setMinimumSize({5,1});
        labelDim.setMaximumSize(30, 1);
        labelDim.setMaximumHeight({30,1});

        addTimer(100);
    }

    void onTimer (fc::FTimerEvent *ev) override
    {
        labelDim.getText() << ".";

        //adjustSize();

        redraw();
    }
};

int main(int argc, char **argv)
{
    fc::FApplication app{argc, argv};
    HelloDialog dialog{app};
    dialog.show();

    app.setMainWidget(&dialog);

    return app.exec();
}
gansm commented 4 years ago

Thanks for your detailed description, which helped me to find some problems with the FLabel widget.

1st problem: The widget default size is 1×1 character. This caused an unsigned integer underflow! 2nd problem: I stupidly implemented the code of setMaximumSize() in setMaximumHeight().

I have a version committed (b7639f5) that fixes these problems.

The main problem in your program is that you don't give the FLabel a size. Therefore the set size hints for minimum and maximum are ignored. The widget still has its default size of 1x1 characters. The size hints are only considered in the methods setWidth(), setHeight(), setSize(), and setGeometry().

Here is the code of the method setSize() for explanation:

void FWidget::setSize (const FSize& size, bool adjust)
{
  std::size_t width = size.getWidth();
  std::size_t height = size.getHeight();
  width  = std::min (width,  size_hints.max_width);
  width  = std::max (width,  size_hints.min_width);
  height = std::min (height, size_hints.max_height);
  height = std::max (height, size_hints.min_height);
  [...]
}

Please try my variant with my last commit.

#include <iostream>
#include <final/final.h>

namespace fc = finalcut;

class HelloDialog : public fc::FDialog
{
  fc::FLabel labelDim{this};

public:
  HelloDialog (fc::FWidget& parent)
    : FDialog(&parent)
  {
    setText("Hello Finalcut - Size Issue");
    setPos({10, 3});
    setSize({40, 18});
    labelDim.setFixedSize({getClientWidth(), 1});  // Sets the size hints
    labelDim.setSize({9, 99});  // Sets the label size by using size hints
    addTimer(100);
  }

  void onTimer (fc::FTimerEvent*) override
  {
    labelDim << "x";
    redraw();  // Prints ellipsis ("..") on text overflow
  }
};

int main (int argc, char* argv[])
{
  fc::FApplication app{argc, argv};
  HelloDialog dialog{app};
  dialog.show();
  app.setMainWidget(&dialog);
  return app.exec();
}
terranpro commented 4 years ago

Glad the report could be of help :) Your patch does indeed fix the overflow, but without setting a size it does default to size 1x1 as you mentioned.

I think I misunderstood the purpose of min/max size hints. I had figured there would be a size calculation based on text length up to the maximum size hint, thus no setSize call. From the code snippet you pasted, it looks like Min/Max size hints are just to enforce a range for calls to setSize. This seems useful for resizeable windows/dialogs - using hints to prevent the user from resizing it too small or too large, right?

Looks like setSize is really just what I need to use with a sane value for my particular layout - and not so much using the size hints for labels. Would you agree with that ?

Also not sure if this is a bug or just something users need to be aware of, but if label minSize >= parent dialogs minSize, then there are overflow artifacts drawn. I attached a couple screenshots of what I mean. Seems like child should set minSize to parent's minSize minus the borders, e.g. (2?) for width, and 3 for height (extra +1 for the top title bar).

setMinimumSize({25, 4}) labelDim.setMinimumSize({25,4})

labelbug2

setMinimumSize({15, 4}) labelDim.setMinimumSize({25, 4})

labelbug3

gansm commented 4 years ago

You have already correctly recognized that with setMinimumSize() and setMaximumSize() only a validity range can be defined.

What you are looking for is a dynamically adaptive layout. For this, you can overload the method adjustSize(). See https://github.com/gansm/finalcut/blob/master/doc/first-steps.md#dynamic-layout

Also, you have correctly recognized that a child widget of a dialog with a frame must always be smaller than its parent widget. Therefore, a widget can have a top, right, bottom, and left padding space, which results in a client width or client height.

Width = LeftPadding + ClientWidth + RightPadding Height = TopPadding + ClientHeight + BottomPadding

◄────────── getWidth() ──────────►
┌────────────────────────────────┐
│                                │
│                                │
│◄────── getClientWidth() ──────►│
│                                │
│                                │
└────────────────────────────────┘
▲                                ▲
│                                │
│                                └── getRightPadding()
└─────────────────────────────────── getLeftPadding()

A small example of how you can use adjustSize():

#include <iostream>
#include <final/final.h>

namespace fc = finalcut;

class HelloDialog : public fc::FDialog
{
  fc::FLabel labelDim{this};

public:
  HelloDialog (fc::FWidget& parent)
    : FDialog(&parent)
  {
    setText("Hello Finalcut - Size Issue");
    setPos({10, 3});
    setSize({25, 7});
    setResizeable();
    // Changing the window size by clicking with the mouse
    // on the lower right corner of the window.
    setMinimumSize({25, 7});
    fc::FSize client_size(getClientWidth(), getClientHeight());
    labelDim.setText(fc::FString(40, L'\u263a'));
    labelDim.setMinimumSize(client_size);
    labelDim.setSize({999, 1});
  }

  void adjustSize() override
  {
    FDialog::adjustSize();
    labelDim.setMinimumWidth(getClientWidth());
    labelDim.setWidth(getClientWidth());
  }
};

int main (int argc, char* argv[])
{
  fc::FApplication app{argc, argv};
  HelloDialog dialog{app};
  dialog.show();
  app.setMainWidget(&dialog);
  return app.exec();
}

By the way, I found a small error in the widget code (23ddf5d).

terranpro commented 4 years ago

Great, thanks again. I'll let you close this - I might ask some other questions in the user feedback issue =)

gansm commented 4 years ago

In the meantime I have written a small chapter about the widget layout:     https://github.com/gansm/finalcut/wiki/First-steps#widget-layout