lxn / walk

A Windows GUI toolkit for the Go Programming Language
Other
6.84k stars 884 forks source link

Remove cruft from "DPI Fun" Series #638

Open zx2c4 opened 4 years ago

zx2c4 commented 4 years ago

@rozmansi added lots of stuff and didn't remove old things. He also added things we don't want.

Many functions are now marked as "deprecated." Instead, just delete those entirely, and we'll adjust client code to be correct, if those depreciated ones are truly incorrect.

There are also functions like these that aren't of use, since pixels are going to be square for ever:


// SizeFrom96DPI2 converts from 1/96" units to native pixels. This version assumes different
// horizontal and vertical DPI.
func SizeFrom96DPI2(value Size, dpi Size) Size {
    return Size{
        Width:  scaleInt(value.Width, float64(dpi.Width)/96.0),
        Height: scaleInt(value.Height, float64(dpi.Height)/96.0),
    }
}

// SizeTo96DPI2 converts from native pixels to 1/96" units. This version assumes different
// horizontal and vertical DPI.
func SizeTo96DPI2(value Size, dpi Size) Size {
    return Size{
        Width:  scaleInt(value.Width, 96.0/float64(dpi.Width)),
        Height: scaleInt(value.Height, 96.0/float64(dpi.Height)),
    }
}

"This version assumes different horizontal and vertical DPI." is an overengineered useless assumption that bloats code made for the screen in a win32 context and not for printers or something.

rozmansi commented 4 years ago

I took extra care not to break any backward compatibility with existing clients in my PR... Just feared my PR would never ever be taken seriously if @lxn's own client(s) wouldn't build and work anymore. It should be OK if those functions I marked depreciated are cleaned from the library in the next step.

On the other hand, I do not agree with @zx2c4 assertion using 2-dimensional DPI is "bloat". Vector math is something kids learn in school. My proposal to change dpis data type from int to Size is about making things universal. Every DC in Win32 has LOGPIXELSX and LOGPIXELSY. DPI is a vector, not scalar. It was like this for the past ~20 years. It's Windows 10 that simplified things and assumed uniform DPI. E.g. GetDpiForWindow(). Maybe, one day we'll get a GetDpiForWindowEx() that will switch back to 2D DPIs.

We're all assuming here. @zx2c4 assumes no one will produce a display with non-uniform DPI in the foreseeable future. I assumed it's better to be safe than sorry. See all the code that needs upgrading because people were assuming 96dpi is a norm for at least a decade.

The same goes with assuming icons are always square.

zx2c4 commented 4 years ago

Every DC in Win32 has LOGPIXELSX and LOGPIXELSY. DPI is a vector, not scalar. It was like this for the past ~20 years. It's Windows 10 that simplified things and assumed uniform DPI. E.g. GetDpiForWindow(). Maybe, one day we'll get a GetDpiForWindowEx() that will switch back to 2D DPIs.

Um, no. Windows implemented it like this in the 90s (with printers in mind) and rectified it for the screen case Windows 10. The Windows 10 screen API does not support two dimensional DPIs. If Windows does not support it, we should not support it. Stop wasting CPU cycles with something that is imaginary.

Vector math is something kids learn in school.

Just because you learned something in school does not mean it is a good idea for real software engineering.

If it doesn't exist in reality, either in hardware or in Windows itself, don't bloat walk with it.

zx2c4 commented 4 years ago

I took extra care not to break any backward compatibility with existing clients in my PR... Just feared my PR would never ever be taken seriously if @lxn's own client(s) wouldn't build and work anymore. It should be OK if those functions I marked depreciated are cleaned from the library in the next step.

I think @lxn and I are both okay with drastic changes. We've made several of these non-stop for the last several months. The goal is to get walk working reliably, and until that happens, there's no point in committing to some stable API. Right now we're experimenting with the DPI stuff. Seems like we're finally settling on a good set of solutions. Let's see how far that takes us. Maybe we'll change direction again, but if it looks like this method is here to stay, let's take it to its conclusion. It's easy to revert commits, anyhow.