~eliasnaur/gio-patches

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH gio] app/internal/wm: fixed Windows window sizes

~pierrec
Details
Message ID
<161725750552.12289.9153958785250636787-0@git.sr.ht>
DKIM signature
missing
Download raw message
Patch: +10 -4
From: pierre <pierre.curto@gmail.com>

Window creation on Windows did not take the screen scale into account.

Signed-off-by: pierre <pierre.curto@gmail.com>
---
-

 app/internal/wm/os_windows.go | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/app/internal/wm/os_windows.go b/app/internal/wm/os_windows.go
index f50fa43..f3277aa 100644
--- a/app/internal/wm/os_windows.go
+++ b/app/internal/wm/os_windows.go
@@ -179,9 +179,11 @@ func createNativeWindow(opts *Options) (*window, error) {
	}
	dpi := windows.GetSystemDPI()
	cfg := configForDPI(dpi)
	width := cfg.Px(opts.Width)
	height := cfg.Px(opts.Height)
	wr := windows.Rect{
		Right:  int32(cfg.Px(opts.Width)),
		Bottom: int32(cfg.Px(opts.Height)),
		Right:  int32(width),
		Bottom: int32(height),
	}
	dwStyle := uint32(windows.WS_OVERLAPPEDWINDOW)
	dwExStyle := uint32(windows.WS_EX_APPWINDOW | windows.WS_EX_WINDOWEDGE)
@@ -193,13 +195,17 @@ func createNativeWindow(opts *Options) (*window, error) {
	deltas.width = wr.Right - wr.Left - deltas.width
	deltas.height = wr.Bottom - wr.Top - deltas.height

	// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-createwindowa
	// For overlapped windows, nWidth is either the window's width, in screen coordinates,
	// or CW_USEDEFAULT.
	screenScale := cfg.PxPerDp
	hwnd, err := windows.CreateWindowEx(dwExStyle,
		resources.class,
		opts.Title,
		dwStyle|windows.WS_CLIPSIBLINGS|windows.WS_CLIPCHILDREN,
		windows.CW_USEDEFAULT, windows.CW_USEDEFAULT,
		wr.Right-wr.Left,
		wr.Bottom-wr.Top,
		int32(float32(width)/screenScale)+deltas.width,
		int32(float32(height)/screenScale)+deltas.height,
		0,
		0,
		resources.handle,
-- 
2.30.2

[gio/patches] build success

builds.sr.ht
Details
Message ID
<CAC6ED6BZNBS.3RE3EK3KDHK37@cirno2>
In-Reply-To
<161725750552.12289.9153958785250636787-0@git.sr.ht> (view parent)
DKIM signature
missing
Download raw message
gio/patches: SUCCESS in 21m35s

[app/internal/wm: fixed Windows window sizes][0] from [~pierrec][1]

[0]: https://lists.sr.ht/~eliasnaur/gio-patches/patches/21701
[1]: mailto:pierre.curto@gmail.com

✓ #474614 SUCCESS gio/patches/linux.yml   https://builds.sr.ht/~eliasnaur/job/474614
✓ #474612 SUCCESS gio/patches/apple.yml   https://builds.sr.ht/~eliasnaur/job/474612
✓ #474615 SUCCESS gio/patches/openbsd.yml https://builds.sr.ht/~eliasnaur/job/474615
✓ #474613 SUCCESS gio/patches/freebsd.yml https://builds.sr.ht/~eliasnaur/job/474613
Details
Message ID
<CAC9MCB498H7.QTJX316UDPAN@themachine>
In-Reply-To
<161725750552.12289.9153958785250636787-0@git.sr.ht> (view parent)
DKIM signature
fail
Download raw message
DKIM signature: fail
On Thu Apr 1, 2021 at 08:10, ~pierrec wrote:
> From: pierre <pierre.curto@gmail.com>
>
> Window creation on Windows did not take the screen scale into account.
>
> Signed-off-by: pierre <pierre.curto@gmail.com>
> ---
> -
>
>  app/internal/wm/os_windows.go | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/app/internal/wm/os_windows.go b/app/internal/wm/os_windows.go
> index f50fa43..f3277aa 100644
> --- a/app/internal/wm/os_windows.go
> +++ b/app/internal/wm/os_windows.go
> @@ -179,9 +179,11 @@ func createNativeWindow(opts *Options) (*window, error) {
>  	}
>  	dpi := windows.GetSystemDPI()
>  	cfg := configForDPI(dpi)
> +	width := cfg.Px(opts.Width)
> +	height := cfg.Px(opts.Height)
>  	wr := windows.Rect{
> -		Right:  int32(cfg.Px(opts.Width)),
> -		Bottom: int32(cfg.Px(opts.Height)),
> +		Right:  int32(width),
> +		Bottom: int32(height),
>  	}
>  	dwStyle := uint32(windows.WS_OVERLAPPEDWINDOW)
>  	dwExStyle := uint32(windows.WS_EX_APPWINDOW | windows.WS_EX_WINDOWEDGE)
> @@ -193,13 +195,17 @@ func createNativeWindow(opts *Options) (*window, error) {
>  	deltas.width = wr.Right - wr.Left - deltas.width
>  	deltas.height = wr.Bottom - wr.Top - deltas.height
>  
> +	// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-createwindowa
> +	// For overlapped windows, nWidth is either the window's width, in screen coordinates,
> +	// or CW_USEDEFAULT.
> +	screenScale := cfg.PxPerDp
>  	hwnd, err := windows.CreateWindowEx(dwExStyle,
>  		resources.class,
>  		opts.Title,
>  		dwStyle|windows.WS_CLIPSIBLINGS|windows.WS_CLIPCHILDREN,
>  		windows.CW_USEDEFAULT, windows.CW_USEDEFAULT,
> -		wr.Right-wr.Left,
> -		wr.Bottom-wr.Top,
> +		int32(float32(width)/screenScale)+deltas.width,
> +		int32(float32(height)/screenScale)+deltas.height,

CreateWindowExW documentation states that its nWidth, nHeight parameters
is "in device units" which I read as pixels. I don't understand why you
need to convert width and height here, which are both in pixels.

Even if you do need to convert them, why not convert them for
AdjustWindowRectEx above?

>  		0,
>  		0,
>  		resources.handle,
> -- 
> 2.30.2
Details
Message ID
<CAG3idScA3AKWCS9EfsCCtGGi-dxXjvMFfNO4f54=hnradL9Hjg@mail.gmail.com>
In-Reply-To
<CAC9MCB498H7.QTJX316UDPAN@themachine> (view parent)
DKIM signature
pass
Download raw message
Let' backtrack a little.
I made a change in os_macos.go that is confusing me.

The screen scaling in os_macos.go/NewWindow will always do nothing:
```
screenScale := float32(C.gio_getScreenBackingScale())
cfg := configFor(screenScale)
...
width = int(float32(cfg.Px(o.Width)) / screenScale)
```

as it is equivalent to:
```
width = int(o.Width.V)
```

right?

I changed (an error on my side)
cfg := configFor(screenScale)

to

cfg := configFor(w.scale)

which *does* have an effect as the view backing scale factor is
different from the screen one.

Now, I am confused:
in window.draw(), w.scale is used (the view scale factor) NOT the
screen one, and in
NewWindow it is the screen factor, which is a noop.

What am I missing?

Le jeu. 1 avr. 2021 à 11:04, Elias Naur <mail@eliasnaur.com> a écrit :
>
> On Thu Apr 1, 2021 at 08:10, ~pierrec wrote:
> > From: pierre <pierre.curto@gmail.com>
> >
> > Window creation on Windows did not take the screen scale into account.
> >
> > Signed-off-by: pierre <pierre.curto@gmail.com>
> > ---
> > -
> >
> >  app/internal/wm/os_windows.go | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/internal/wm/os_windows.go b/app/internal/wm/os_windows.go
> > index f50fa43..f3277aa 100644
> > --- a/app/internal/wm/os_windows.go
> > +++ b/app/internal/wm/os_windows.go
> > @@ -179,9 +179,11 @@ func createNativeWindow(opts *Options) (*window, error) {
> >       }
> >       dpi := windows.GetSystemDPI()
> >       cfg := configForDPI(dpi)
> > +     width := cfg.Px(opts.Width)
> > +     height := cfg.Px(opts.Height)
> >       wr := windows.Rect{
> > -             Right:  int32(cfg.Px(opts.Width)),
> > -             Bottom: int32(cfg.Px(opts.Height)),
> > +             Right:  int32(width),
> > +             Bottom: int32(height),
> >       }
> >       dwStyle := uint32(windows.WS_OVERLAPPEDWINDOW)
> >       dwExStyle := uint32(windows.WS_EX_APPWINDOW | windows.WS_EX_WINDOWEDGE)
> > @@ -193,13 +195,17 @@ func createNativeWindow(opts *Options) (*window, error) {
> >       deltas.width = wr.Right - wr.Left - deltas.width
> >       deltas.height = wr.Bottom - wr.Top - deltas.height
> >
> > +     // https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-createwindowa
> > +     // For overlapped windows, nWidth is either the window's width, in screen coordinates,
> > +     // or CW_USEDEFAULT.
> > +     screenScale := cfg.PxPerDp
> >       hwnd, err := windows.CreateWindowEx(dwExStyle,
> >               resources.class,
> >               opts.Title,
> >               dwStyle|windows.WS_CLIPSIBLINGS|windows.WS_CLIPCHILDREN,
> >               windows.CW_USEDEFAULT, windows.CW_USEDEFAULT,
> > -             wr.Right-wr.Left,
> > -             wr.Bottom-wr.Top,
> > +             int32(float32(width)/screenScale)+deltas.width,
> > +             int32(float32(height)/screenScale)+deltas.height,
>
> CreateWindowExW documentation states that its nWidth, nHeight parameters
> is "in device units" which I read as pixels. I don't understand why you
> need to convert width and height here, which are both in pixels.
>
> Even if you do need to convert them, why not convert them for
> AdjustWindowRectEx above?
>
> >               0,
> >               0,
> >               resources.handle,
> > --
> > 2.30.2
>
Details
Message ID
<CACHAYNUGCPO.186LVHOWC4L8C@themachine>
In-Reply-To
<CAG3idScA3AKWCS9EfsCCtGGi-dxXjvMFfNO4f54=hnradL9Hjg@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
On Thu Apr 1, 2021 at 12:06, Pierre Curto wrote:
> Let' backtrack a little.
> I made a change in os_macos.go that is confusing me.
>
> The screen scaling in os_macos.go/NewWindow will always do nothing:
> ```
> screenScale := float32(C.gio_getScreenBackingScale())
> cfg := configFor(screenScale)
> ...
> width = int(float32(cfg.Px(o.Width)) / screenScale)
> ```
>
> as it is equivalent to:
> ```
> width = int(o.Width.V)
> ```
>
> right?
>

If the user passed in a unit.Dp, but not for unit.Px. And not for
unit.Sp if Gio is changed to accomodate the system text scale as it does
on Android and iOS.

> I changed (an error on my side)
> cfg := configFor(screenScale)
>
> to
>
> cfg := configFor(w.scale)
>
> which *does* have an effect as the view backing scale factor is
> different from the screen one.
>
> Now, I am confused:
> in window.draw(), w.scale is used (the view scale factor) NOT the
> screen one, and in
> NewWindow it is the screen factor, which is a noop.
>
> What am I missing?
>

Gio operates in device pixels, [NSWindow initWithContentRect:...] in
screen pixels.

Elias
Reply to thread Export thread (mbox)