Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FLabel Text Wrapping, Not Obeying Max Size Hint #43

Closed
terranpro opened this issue Dec 20, 2019 · 5 comments
Closed

FLabel Text Wrapping, Not Obeying Max Size Hint #43

terranpro opened this issue Dec 20, 2019 · 5 comments

Comments

@terranpro
Copy link

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
Copy link
Owner

gansm commented Dec 20, 2019

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
Copy link
Author

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
Copy link
Owner

gansm commented Dec 23, 2019

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
Copy link
Author

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

@gansm gansm closed this as completed Dec 24, 2019
@gansm
Copy link
Owner

gansm commented Jan 14, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants