Re: [PATCH] HiDPI fixes for squiggly underlines

Hi,

On Fri, Dec 13, 2013 at 9:58 PM, Keith Curtis <keithcu@gmail.com> wrote:
> Hi Kendy,
>
> Good to hear from you. I've got a number of things in progress on my
> computer beyond the underlines
> (https://wiki.documentfoundation.org/Development/HiDpi) but I wait to get an
> API first as I'm just writing "if (1) //hidpi".
>
> I personally don't think a floating point value is a good idea. At the
> lowest level, pixel line widths go to 2 or 3 and in the square case go to 4
> or 9. I believe anything who wants more resolution should probably be
> working based on the system font size. The changes I'm making don't seem to
> need more than 3 and even 2 is plenty for all laptops today and into the
> distant future. Floating point arithmetic used to be a lot slower than
> integer as well but I don't know if that is still true ;-)
>
> I hope it is okay if I can write code that handles the 2x case but has no
> guarantees about any other value. I can't test what I can't see and there is
> a fair amount of work to get LibreOffice looking good and supporting
> variable-sized bitmaps at all. So if it were up to me, I'd just make it a
> boolean for a while, and focus / force that part to look great everywhere.
> Then when you were ready to further generalize it, the compiler would make
> you to validate all the areas. I even have been taking advantage of the
> simplicity. I found some arrow drawing logic that wanted an odd-sized height
> for best results. And so I just doubled it and subtracted one ;-)
>
> My biggest concern wrt the API is that it has the right value out of the
> box. For example, what do normal Macs return for the system font size versus
> Retinas? What about on Windows? There is plenty of intelligence that can go
> into a bit.
>
> Putting the change in ImplDrawWaveLine is possible. My change is safer in
> that it mostly only touches the spelling / grammar which is the most
> noticeable issue. With ImplDrawWaveLine you have to worry about more
> callers, printers, etc. and possibilities for dirt on screen. I wouldn't
> necessarily recommend shifting down 2 pixels for the low-level drawing code,
> so maybe you'd still have to have changes in both places. ImplDrawWaveLine
> is a routine takes a pixel line width as input, so perhaps the policy
> decision should be above it. It seems sort of like putting the bitmap
> doubling logic into the BitBlt routine versus in the toolbar which loads the
> bitmaps. I could look at the wavy underline character property logic, but I
> think it is low priority and still might not change the ImplDrawWaveLine. I
> haven't looked into it. I'm happy to fix any bugs in my simple code, but get
> concerned about suggestions that complicate it ;-)
>
> Working on this early next week would be great. I've got about 600 more
> lines in 14 files to review that make a difference. There is a fair amount
> of low-hanging fruit. I am possibly stuck on two issues, but I plug away on
> it as I have free time. The hardest part is finding the actual line of code
> that have the bug. With my work, they are being marked which makes future
> work easier. I did see code that is possibly problematic in places like
> DecoView but I'm just focusing on the visible ones. Toolbar bitmaps are the
> most important. The Sidebar is the most broken. Do you think they will fix
> it?
>
> Anyway, I look forward to working through the issues soon because I have
> some time now and hopefully getting this into 4.2.
>
> -Keith

Great work. But IMHO this doesn't really solve the problem - or at
least I think this approach is not correct for canvas elements like
wave lines. Everything drawn on canvas should be defined in absolute
units (100th mm or similar) and not pixels. If I set the width of a
wave line to 2 mm I expect that when the monitor DPI = system DPI and
the zoom level is 100%, that I can take the ruler and measure the wave
line width to be exactly 2 mm (and 4 mm at zoom 200%, 8 mm at zoom
400%, ...). This must be true for all elements on the canvas at least
(for toolbars and non canvas elements IMHO the same rule should apply
but this requires then a resolution independent backend).

Regards, Tomaž