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

Autosize of windows is not work properly on different DPI scalings #677

Open
Alexpux opened this issue Jul 14, 2023 · 35 comments
Open

Autosize of windows is not work properly on different DPI scalings #677

Alexpux opened this issue Jul 14, 2023 · 35 comments

Comments

@Alexpux
Copy link

Alexpux commented Jul 14, 2023

On Win10 when you change scaling factor in settings layouts autosize not work properly. From demo projects here is inputbox pictures for 100%, 125% and 150% scaling:
right_layout_size
wrong_layout_size
wrong_layout_size_150

go to: #698

@cnjinhao
Copy link
Owner

Thank you for reporting this issue. I will check it.

@edgarbernal5
Copy link

Hi! I don't know if it is the same issue but when I moved the form created in a 4k monitor to a HD monitor the title bar (caption, minimize, maximize, etc) won't scale the title bar correctly.

4K monitor
nana test 4k

HD monitor
nana test hd

@edgarbernal5
Copy link

Also I managed to scale menubar with different DPI by getting widget size of it. Here is the code within source/gui/widgets/menubar.cpp

std::size_t find(const ::nana::point& pos)
{
  // changed this line:
  if ((2 <= pos.x) && (2 <= pos.y) && (pos.y < 25))
  
  //with this:
  auto menubar_size = widget_ptr->size();
  if ((2 <= pos.x) && (2 <= pos.y) && (pos.y < menubar_size.height))

....

Also on refresh method

void trigger::refresh(graph_reference graph)
{
....
  auto menubar_size = ess_->widget_ptr->size();
  nana::point item_pos(2, 2);
  nana::size item_s(0, menubar_size.height - 2);
...

What do you think? Thanks in advance.
There is still a bug with menu popoups

@cnjinhao
Copy link
Owner

@edgarbernal5 a PR is welcomed.

@edgarbernal5
Copy link

Hi @cnjinhao, I want to contribute, I have a commit ready to be pushed but I think I don't have right access:

git push --set-upstream origin dpi_scale_issue
remote: Permission to cnjinhao/nana.git denied to edgarbernal5.
fatal: unable to access 'https://github.com/cnjinhao/nana.git/': The requested URL returned error: 403

@qPCR4vir
Copy link
Collaborator

@edgarbernal5 make your push into your own fork of nana https://github.com/edgarbernal5/nana, and from there open a pull request to the original fork.

@edgarbernal5
Copy link

Alright, I didn't know that! Thanks @qPCR4vir will do that!

@edgarbernal5
Copy link

In order to create the pull request, should I create a branch in my own fork with all in? @qPCR4vir

@edgarbernal5
Copy link

I think I know the answer 😀

@edgarbernal5
Copy link

Pull request: #690.
Any feedback is welcomed.

@cnjinhao
Copy link
Owner

Pull request: #690.

Any feedback is welcomed.

Thanks, the pull request has been merged.

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 2, 2024

Pull request: #690. Any feedback is welcomed.

Hi ! It looks like a great job. I hope you (@edgarbernal5 ) will keep doing soo.

But in your Pull Request you (@edgarbernal5 ) made the same mistake I made some times: there are a lot of unrelated commits that make it very hard to follow what you did. I'm sorry that I did not review that, and only now report it. To fix this I propose that you both go to the previous commit: 7c73095 "fix crash and rectangle errors in pixel_buffer" and make a hard reset of the branch to that. @cnjinhao will need to rewrite that to the repo (force push that branch again). After that one (@edgarbernal5 ) need to do a merge , but using a 'cherry pick' and include only the commit fe75360 "Fixed dpi scale for menu, menubar and combox". In a new Pull request verify only that commit was made. @cnjinhao will then merged and made a new push. This will re-write history. A clean history. There are many ways to do this, just verify before push. If you want I can try to do all of that by myself. We all learn by doing...

image

@edgarbernal5
Copy link

My bad, it was my fault! I want to fix this but I don't know how to do it right! We can do it together if you want @qPCR4vir

I got the following message:

git reset --hard 7c73095
fatal: Could not parse object '7c73095f26e4a7e2b29af6097e692ba7e2cd6d71'.

or doing this:

git checkout 7c73095
fatal: reference is not a tree: 7c73095

Probably I am missing something.
By the way I want to do the same thing in my fork, to clean the mess, because this project is awesome and want to continue developing and contributing not only with bug fixing but also with creating new widgets.

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 5, 2024

ok... I did what I could.. I hope it is not too bad now.
@cnjinhao and @edgarbernal5 will need to rename your local branch develop-1.8 and pull it again from this repo.

@edgarbernal5 to clean your repo just rename your develop-1.8 to my_dev-1.8 or something and pull a fresh develop-1.8 from this repo. Then create a new branch directly from that fresh pulled develop-1.8 with the name of the feature or fix you want to provide. Checkout that and cherry pick only the commits that you want to merge. Push to your repo in GitHub and from there make a PullRequest to cnjinhao/develop-1.8. Make sure it is a clean PR, with only the needed commits and changes.
Good luck!

@edgarbernal5
Copy link

edgarbernal5 commented Apr 6, 2024

Thanks a million! You saved me @qPCR4vir !!

@qPCR4vir
Copy link
Collaborator

Hi, @edgarbernal5 great job!....
now, could you try to fix the original post here about nana::inputbox?

@qPCR4vir qPCR4vir added the bug label Apr 10, 2024
@qPCR4vir
Copy link
Collaborator

tip: begin from this :

	bool inputbox::_m_open(std::vector<abstract_content*>& contents, bool modal)
	{
		std::vector<unsigned> each_pixels;
		unsigned label_px = 0, fixed_px = 0;
		paint::graphics graph({ 5, 5 });

		bool has_0_fixed_px = false;
		for (auto p : contents)
		{
			auto px = label::measure(graph, p->label(), 150, true, align::right, align_v::center);
			if (px.width > label_px)
				label_px = px.width;

			px.width = p->fixed_pixels();
			has_0_fixed_px |= (px.width == 0);
			if (px.width > fixed_px)
				fixed_px = px.width;

			each_pixels.push_back(px.height);
		}

		//Ensure that the entry fields are at least as wide as the minimum
		if (has_0_fixed_px && (fixed_px < min_width_entry_field_pixels_ ))
			fixed_px = min_width_entry_field_pixels_;

		inputbox_window input_wd(owner_, images_, valid_areas_, description_, title_, contents.size(), label_px + 10 + fixed_px, each_pixels);

		std::vector<window> inputs;
		for (auto p : contents)
			inputs.push_back(p->create(input_wd, label_px));

		input_wd.set_input(inputs, verifier_);

		if (modal)
			input_wd.modality();
		else
			api::wait_for(input_wd);

		return input_wd.valid_input();
	}

@qPCR4vir
Copy link
Collaborator

still a lot of problems like this: just set it to 300% for easy detection of problems

@edgarbernal5
Copy link

edgarbernal5 commented Apr 11, 2024

@qPCR4vir Sure, no problem. I could take a look. Would you know what are the repro steps?

still a lot of problems like this: just set it to 300% for easy detection of problems

Do you mean changing scale and layout on display on Windows Settings?

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 11, 2024

On Win10 when you change scaling factor in settings layouts autosize not work properly. From demo projects here is inputbox pictures for 100%, 125% and 150% scaling:

Yes, with 300% it is easy to see....
And directly from the provided pictures, it seems to me that the major problem is in the calculation and setting of the size of the form inputbox_window input_wd

The constructor of inputbox_window in nana\source\gui\msgbox.cpp end with:
move(api::make_center(this->owner(), description_size.width, height));
And both the width and the height are bad (too small) by scaling other than 100%.

@qPCR4vir
Copy link
Collaborator

Unfortunately, the listbox widget is also affected.

@edgarbernal5
Copy link

The constructor of inputbox_window in nana\source\gui\msgbox.cpp end with: move(api::make_center(this->owner(), description_size.width, height)); And both the width and the height are bad (too small) by scaling other than 100%.

Okey, got it. Maybe we need to do the dpi scale by calling this:
move(api::make_center(this->owner(), platform_abstraction::dpi_scale(handle(), description_size.width), platform_abstraction::dpi_scale(handle(), height)));

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 12, 2024

Well... I wanted to help with this, but I'm afraid I don't understand why we are doing this:
when I want to set a scale, I go to Windows settings and change the scale for that monitor, and ALL programs just rescale the ENTIRE drawing. Why are we trying to do that separately in nana? I don't expect and don't want any program to rewrite my settings and try to change the scale on its own... or... what am I missing here?
What are we trying to do? If your operating system has no way to rescale a high dpi monitor... well, you need a new operating system. But how will a program guess what the user wants?
I use a 4k monitor (actually a big and cheap Samsung TV), and set 100% in windows setting: all OK, inlcusive nana.
But if I set 200% - all program are now big, but nana only partially. Why?
Normally I use 100%... but what if one day I want to do a presentation at 200%.. and all program will nicely scale, but not my program with nana?
What am I missing here? At some point did operating systems offer non-scaling, making high dpi monitors useless? And nana 'fixed' that?
Or, in order to scale, the programs need to do something apart to just redraw everything?

@qPCR4vir
Copy link
Collaborator

hmm, #640...

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 12, 2024

OK, I suppose that the goal was to "amplify" extreme small fonts and elements to make it readable for dpi > 96.
I will try to 'fix' all of this by preventing scaling when the used dpi is less than 96 and scaling only when it is >96.

@qPCR4vir
Copy link
Collaborator

Why for font it is dpi / 72 and for other use dpi / 96 ?
hmmm, found: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-logfontw

@qPCR4vir
Copy link
Collaborator

testing: https://github.com/qPCR4vir/nana/tree/dpi

@qPCR4vir
Copy link
Collaborator

hmm, no. Windows internally reinterpret all the coordinates, scaling for that % from the windows setting and report the new dpi already "scaled", but your program don't see that - it continue to to use the same old coordinates. We need to know when it was set for a physical monitor or for an 'scaled' monitor.

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 15, 2024

Sad... I'm not doing any progress. We need a lot of discipline to fix this.
Somewhere we are bypassing/duplicating the dpi_scaling: font? windows size? measure? case WM_DPICHANGED?

@edgarbernal5
Copy link

Maybe it is a good idea if we readjust size and position of all widget of root widgets upon WM_DPICHANGED message arrives. What do you guys think about it? @cnjinhao @qPCR4vir

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Apr 23, 2024

OK... as per this issue, we want to adapt to a dpi change: we need DPI_AWARENESS_PER_MONITOR_AWARE (= 2, version 2)
We need to use GetDpiForWindow
It will not work in wind7.
int scaled_x = x_opt4_96dpi * dpi / USER_DEFAULT_SCREEN_DPI;
USER_DEFAULT_SCREEN_DPI = 96

OMG... we need to "manually" calculate and scale every single coordinate, size and font? well, windows rectangle not - only set it, but windows give the new coordinate already calculated.
As I see we need to to that in inputbox and in listbox.

@qPCR4vir
Copy link
Collaborator

please see: #697

@qPCR4vir
Copy link
Collaborator

go to: #698

@cnjinhao
Copy link
Owner

cnjinhao commented May 2, 2024

The current design is not easy for resizing window of DPI change. We need to design an adaptive coordinating system for the feature, it's complicated. IMO, we can add the event of DPI change to let user control the resize.

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

4 participants