Hi-DPI patches for 4.2

classic Classic list List threaded Threaded
19 messages Options
Jan Holesovsky-4 Jan Holesovsky-4
Reply | Threaded
Open this post in threaded view
|

Hi-DPI patches for 4.2

Hi Caolán, all,

I've just pushed a backport of the hi-dpi patches from master to gerrit
for libreoffice-4-2 integration - as was requested earlier, to fix the
unfortunate state of LibreOffice on the hi-dpi displays.  It is the
following 5 patches (order is important):

https://gerrit.libreoffice.org/#/c/8516/
https://gerrit.libreoffice.org/#/c/8517/
https://gerrit.libreoffice.org/#/c/8518/
https://gerrit.libreoffice.org/#/c/8519/
https://gerrit.libreoffice.org/#/c/8520/

Keith confirmed that they fix the hi-dpi issues he was seeing in
LibreOffice 4.2.

They are supposed to be safe for normal displays; that is anything
non-safe should be enclosed in an "if (mnDPIScaleFactor > 1)".  Few
cases make the computation a bit more general, like:

+    long yOffset = (aRect.GetHeight() - mpImpl->maImage.GetSizePixel().Height()) / 2;
+
     if( mpImpl->mnState == SIGNATURESTATE_SIGNATURES_OK )
     {
-        ++aRect.Top();
+        aRect.Top() += yOffset;

etc. but hopefully such places are safe enough too.

It is only 5 patches, as I have squashed the follow-up fixes into the
appropriate patches, but other than that, it should bring us to the very
same state that is in master.

Thank you in advance,
Kendy

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Mon, Mar 10, 2014 at 1:24 PM, Jan Holesovsky <[hidden email]> wrote:
> Hi Caolán, all,
>

>
> It is only 5 patches, as I have squashed the follow-up fixes into the
> appropriate patches, but other than that, it should bring us to the very
> same state that is in master.

Not quite.. I had to add a bunch of #ifndef MACOSX around most of it
(and I may have missed some)
so that it does not mess-up Macosx retina display support.
especially wrt to code that 'scale' BMPs....
as such I'm quite nervous to have that backported in the 'stable' release...

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Mon, Mar 10, 2014 at 2:11 PM, Norbert Thiebaud <[hidden email]> wrote:

> On Mon, Mar 10, 2014 at 1:24 PM, Jan Holesovsky <[hidden email]> wrote:
>> Hi Caolán, all,
>>
>
>>
>> It is only 5 patches, as I have squashed the follow-up fixes into the
>> appropriate patches, but other than that, it should bring us to the very
>> same state that is in master.
>
> Not quite.. I had to add a bunch of #ifndef MACOSX around most of it
> (and I may have missed some)
> so that it does not mess-up Macosx retina display support.

see core:2a0a80a1385544cbb8d9f6b3ffc22f1c0afeb4ed
for example
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Jan Holesovsky-4 Jan Holesovsky-4
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

In reply to this post by Norbert Thiebaud
Hi Norbert,

Norbert Thiebaud píše v Po 10. 03. 2014 v 14:11 -0500:

> > It is only 5 patches, as I have squashed the follow-up fixes into the
> > appropriate patches, but other than that, it should bring us to the very
> > same state that is in master.
>
> Not quite.. I had to add a bunch of #ifndef MACOSX around most of it
> (and I may have missed some)
> so that it does not mess-up Macosx retina display support.

I was assured that those ifdefs are needed only when

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0c9e4d9b223a0593686bee800484de3c23095d4f

is in place; and I did not backport that one.  So does it kick in even
if this is not there?

> especially wrt to code that 'scale' BMPs....
> as such I'm quite nervous to have that backported in the 'stable' release...

Well - no idea what is best there; either the bitmaps are extremely
small, but not scaled, or they have reasonable size, but every pixel
becomes a square...

I for myself am OK to live with the scaled bitmaps before we have an
icon set that either has bigger png's, or directly svg's, but that is I
guess to decide between people who actually have hi-dpi displays :-)

Having said that - why ifdef? ;-)  Why just not a function that does (or
does not) scale according to the DPI scale level & if it is / is not
OSX?

All the best,
Kendy

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Mon, Mar 10, 2014 at 2:32 PM, Jan Holesovsky <[hidden email]> wrote:

> Hi Norbert,
>
> Norbert Thiebaud píše v Po 10. 03. 2014 v 14:11 -0500:
>
>> > It is only 5 patches, as I have squashed the follow-up fixes into the
>> > appropriate patches, but other than that, it should bring us to the very
>> > same state that is in master.
>>
>> Not quite.. I had to add a bunch of #ifndef MACOSX around most of it
>> (and I may have missed some)
>> so that it does not mess-up Macosx retina display support.
>
> I was assured that those ifdefs are needed only when
>
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=0c9e4d9b223a0593686bee800484de3c23095d4f
>

I'll double check if that is indeed correct.
othoh if you have a 'staging' branch for these.. I may just try to
build _that_ and report.


>
> Having said that - why ifdef? ;-)  Why just not a function that does (or
> does not) scale according to the DPI scale level & if it is / is not
> OSX?

because 1/ that makes it clear that it is a undesirable hack (ifdef
stick out has ugly thing :-) )
2/ it may not be possible to address all these in one pass...
The problem I have not yet figured-out entirely is how properly deal
with size of the bitmap as expressed in 'point'
and size of he bitmap as expressed in 'pixel'....
and some bmp are 'resource' and other are 'generated at runtime (like
preview)... these pose a different set of problems.
3/ macosx magically deal with the scaling by default.. in fact _not_
making it do that for bitmap turned out to be something I have not
figured out yet...
/me has bitten the bullet and bought a couple of book about coca and
quartz programming.. with the hope to gain enough understanding of the
beast to be able to move vcl toward an api that would support some
control over these...

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Keith Curtis Keith Curtis
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

Perhaps it would be good to have Norbert double-check his work, but it
seems logical the later changes aren't necessary without his first
retina change to turn off auto-doubling mode, which is not a part of
this set.

Norbert is still in the investigation phase, and there are multiple
ways to implement this on the Mac, so there is no need to optimize for
cleanliness until it works. It should be put in the release notes that
this is for Linux and Windows only as Mac will still run in
compatibility mode.

Doubling bitmaps is a "hack" but since bigger bitmaps don't exist, it
is better than doing nothing. I haven't looked into the low-level
resource loading code, but there are very probably VCL changes
required once those new bitmaps are created. Once that happens, then
the doubling code can be removed, but only at the end, and it might be
a while given how many bitmaps exist in all the icon packs out there.

-Keith
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Mon, Mar 10, 2014 at 4:51 PM, Keith Curtis <[hidden email]> wrote:
> Doubling bitmaps is a "hack" but since bigger bitmaps don't exist, it
> is better than doing nothing.

The problem is that mac, in compatibility mode, already auto-double..
so we end-up with a quadrupling
of the bitmap, which mean you in the end only see a quarter of the
intended icon.
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Keith Curtis Keith Curtis
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Mon, Mar 10, 2014 at 7:15 PM, Norbert Thiebaud <[hidden email]> wrote:
> On Mon, Mar 10, 2014 at 4:51 PM, Keith Curtis <[hidden email]> wrote:
>> Doubling bitmaps is a "hack" but since bigger bitmaps don't exist, it
>> is better than doing nothing.
>
> The problem is that mac, in compatibility mode, already auto-double..
> so we end-up with a quadrupling
> of the bitmap, which mean you in the end only see a quarter of the
> intended icon.

Have you made a build with it turned back on? I ask because it doesn't
make sense the OS would return DPI != 96 while in auto-doubling mode.
That would seemingly break the backward compatibility support the Mac
is trying to achieve.

Thanks,

-Keith
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2



On Mon, Mar 10, 2014 at 8:11 PM, Keith Curtis <[hidden email]> wrote:
> On Mon, Mar 10, 2014 at 7:15 PM, Norbert Thiebaud <[hidden email]> wrote:
>> On Mon, Mar 10, 2014 at 4:51 PM, Keith Curtis <[hidden email]> wrote:
>>> Doubling bitmaps is a "hack" but since bigger bitmaps don't exist, it
>>> is better than doing nothing.
>>
>> The problem is that mac, in compatibility mode, already auto-double..
>> so we end-up with a quadrupling
>> of the bitmap, which mean you in the end only see a quarter of the
>> intended icon.
>
> Have you made a build with it turned back on? I ask because it doesn't
> make sense the OS would return DPI != 96 while in auto-doubling mode.
> That would seemingly break the backward compatibility support the Mac
> is trying to achieve.

a picture is worth a 1000 words
_with_ the #ifdef MACOSX removed and _without_ --enable-retina (I could take a snapshot of the Info page of the app that show the 'open in low-resolution' checked and grey-ed out...)


Inline image 1

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Stefan Knorr Stefan Knorr
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

In reply to this post by Keith Curtis
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi all,

On 10/03/14 22:51, Keith Curtis wrote:
> Doubling bitmaps is a "hack" but since bigger bitmaps don't exist, it
> is better than doing nothing. I haven't looked into the low-level
> resource loading code, but there are very probably VCL changes
> required once those new bitmaps are created. Once that happens, then
> the doubling code can be removed, but only at the end, and it might be
> a while given how many bitmaps exist in all the icon packs out there.

Just fyi, that is https://bugs.freedesktop.org/show_bug.cgi?id=51733 .
Proposals by developers on how to add pixel-doubled bitmaps to existing
themes/create new pixel-doubled themes welcome.
Also, as long as we ship all those themes that nobody can really do much
about  without completely recreating them (Galaxy e.g.), the
pixel-doubling code would need to stay.


Astron.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTHthZAAoJEJCfzwJOvloORiIH/1/rDMdRZLfHFjdKuHy0tFVP
jIzHTkbTVz0XRaJnx5dS1VzE2Sc1ts3pAcCo1xbzGxl7R2Ywhgbuq/jIZf9gV9A+
f954AhL3VYdSuzKu4tr/qDwp7CIDreIDqhsb0nguV9pKMZyYIIRjLx0kXXCYUIh8
4IWw2RHymjvaOu22CvpHD4Ro6AJj2BvN2QnkOyXWxlqwPVCl64lnKr8RDQTpdl9L
8bhgA+xSDqmMPVE4RQYJdKsxTTZuwZu762j4SwWe2EY9VrQoHnfj/MGY84pIycnb
QQJkoseMPS/55xlnSfLKCT/a9/xJ1MKiGy3Q1Xa/zXwx4EPFt4dyYEu4Gh1nGxQ=
=sMWV
-----END PGP SIGNATURE-----
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Khaled Hosny Khaled Hosny
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

CONTENTS DELETED
The author has deleted this message.
Keith Curtis Keith Curtis
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

In reply to this post by Norbert Thiebaud
That picture is bizarre in that the gridlines drawn by Calc are not being doubled. Have you fiddled with the OS DPI stuff?

-Keith


On Tue, Mar 11, 2014 at 3:17 AM, Norbert Thiebaud <[hidden email]> wrote:


On Mon, Mar 10, 2014 at 8:11 PM, Keith Curtis <[hidden email]> wrote:
> On Mon, Mar 10, 2014 at 7:15 PM, Norbert Thiebaud <[hidden email]> wrote:
>> On Mon, Mar 10, 2014 at 4:51 PM, Keith Curtis <[hidden email]> wrote:
>>> Doubling bitmaps is a "hack" but since bigger bitmaps don't exist, it
>>> is better than doing nothing.
>>
>> The problem is that mac, in compatibility mode, already auto-double..
>> so we end-up with a quadrupling
>> of the bitmap, which mean you in the end only see a quarter of the
>> intended icon.
>
> Have you made a build with it turned back on? I ask because it doesn't
> make sense the OS would return DPI != 96 while in auto-doubling mode.
> That would seemingly break the backward compatibility support the Mac
> is trying to achieve.

a picture is worth a 1000 words
_with_ the #ifdef MACOSX removed and _without_ --enable-retina (I could take a snapshot of the Info page of the app that show the 'open in low-resolution' checked and grey-ed out...)


Inline image 1


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2



On Tue, Mar 11, 2014 at 3:53 AM, Keith Curtis <[hidden email]> wrote:
That picture is bizarre in that the gridlines drawn by Calc are not being doubled. Have you fiddled with the OS DPI stuff?

The grid lines are not bitmap, they are vector drawing...
 but more importantly... _WITH_ the #ifdef to prevent the meddling with bitmap everything looks 'fine'.



_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Keith Curtis Keith Curtis
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Tue, Mar 11, 2014 at 4:57 AM, Norbert Thiebaud <[hidden email]> wrote:

>
>
> On Tue, Mar 11, 2014 at 3:53 AM, Keith Curtis <[hidden email]> wrote:
>>
>> That picture is bizarre in that the gridlines drawn by Calc are not being
>> doubled. Have you fiddled with the OS DPI stuff?
>
>
> The grid lines are not bitmap, they are vector drawing...
>  but more importantly... _WITH_ the #ifdef to prevent the meddling with
> bitmap everything looks 'fine'.
>
>

Yes, but according to the docs, in compatibility mode, "Any
vector-based drawing performed by an app is scaled for high
resolution"

https://developer.apple.com/library/mac/documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/Explained/Explained.html#//apple_ref/doc/uid/TP40012302-CH4-SW5

The Mac LibreOffice screenshots in bug reports I have seen have
doubled vector drawing. I think there is something wrong with your
build. In any case, if you feel something should be done for the Mac,
the simplest thing is to add two #ifdefs to force the DPIScaleFactor
to 1 for the Mac:

https://gerrit.libreoffice.org/#/c/8516/1/vcl/source/window/window.cxx,cm

That would turn off all bitmap doubling in a very localized place.
What do you think about that?

-Keith
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Keith Curtis Keith Curtis
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

Well, the rabbit hole gets deeper. Norbert, it now seems to me that there isn't something wrong with your build, but with your OS!?!

Here is a screenshot of the current released build on Retina for comparison:


It generally looks fine, and doesn't need improvements. In it, you can see that the Calc gridlines are two pixels wide as expected because of the app scaling. (For comparison, on Linux, they are always one pixel wide which is perhaps not ideal but looks okay.)

However, the gridlines are effectively the same width as yours. But look at the close buttons. I've zoomed in both to make it clearer. The one on the left is from the screenshot above, and the one on the right is from your screenshot last night.



It seems as if all of Norbert's pixels are doubled. How does this relate to the compatibility mode and DPI scale factor? I don't know WTF is going on and it is time-consuming without any hardware to try things. It seems there are possibly two problems on Norbert's machine.

If the OS really gives the wrong DPI even in compatibility mode, which doesn't seem correct but is of course possible, then the simplest way disable these patches on the Mac is to just force mnDPIScaleFactor to 1. In that case, these patches would behave as it does on 4.2.1. This can be done with 2 #ifndefs in window.cxx and should achieve a similar result as disabling the bitmap doubling all over the place. Norbert, can you try that?

Mac doesn't need this code as much as Win / Linux so there needs a simple way to decouple them. I was quite certain when Norbert had sent out his first mail to this thread saying there were problems with these patches that he had forgotten about what happens in compatibility mode is on -- because he'd been spending all of his time working with the "off" case. But I never imagined it would be this complicated. It would be nice to have these patches not held up by the Mac as they are right now.

-Keith
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

On Tue, Mar 11, 2014 at 2:32 PM, Keith Curtis <[hidden email]> wrote:
> then the simplest way
> disable these patches on the Mac is to just force mnDPIScaleFactor to 1. In
> that case, these patches would behave as it does on 4.2.1.

yep, that does that indeed.
pushed
http://cgit.freedesktop.org/libreoffice/core/commit/?id=4dbb04e5701efe084fbfd3f06128dd33a7d8965b

which need to be picked-up on 4-2 if your going to backport these
HiDPI patches..

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2


for info here is teh different DPI reported by vcl for the same 13'' screen at different display setting

n_th@Norberts-MacBook-Pro /lo/core$ ./instdir/LibreOfficeDev.app/Contents/MacOS/soffice
2014-03-11 19:39:44.065 soffice[270:507] final mnRealDPIX = 91 mnRealDPIY = 91
n_th@Norberts-MacBook-Pro /lo/core$ ./instdir/LibreOfficeDev.app/Contents/MacOS/soffice
2014-03-11 19:34:11.715 soffice[267:507] final mnRealDPIX = 113 mnRealDPIY = 113
n_th@Norberts-MacBook-Pro /lo/core$ ./instdir/LibreOfficeDev.app/Contents/MacOS/soffice
2014-03-11 19:32:59.299 soffice[251:507] final mnRealDPIX = 128 mnRealDPIY = 128
n_th@Norberts-MacBook-Pro /lo/core$ ./instdir/LibreOfficeDev.app/Contents/MacOS/soffice
2014-03-11 19:33:31.395 soffice[264:507] final mnRealDPIX = 149 mnRealDPIY = 149

That is why the so called 'ScaleFactor' which was calculated with

f = E((dpi + 48) / 96)

gave different behavior at different level of display resolutions.. none of them having anything to do with the retina 'doubling' thing.

The one at 113 is the one said to be best for retina... but in all case 1 point = 4 pixel.. the later two case are 'scaled down to fit the screen' by the system itself.
none of these value are the actual device ppi.


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Jan Holesovsky-4 Jan Holesovsky-4
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

In reply to this post by Norbert Thiebaud
Hi Norbert,

Norbert Thiebaud píše v Út 11. 03. 2014 v 19:29 -0500:

> > then the simplest way
> > disable these patches on the Mac is to just force mnDPIScaleFactor to 1. In
> > that case, these patches would behave as it does on 4.2.1.
>
> yep, that does that indeed.
> pushed
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=4dbb04e5701efe084fbfd3f06128dd33a7d8965b
>
> which need to be picked-up on 4-2 if your going to backport these
> HiDPI patches..

Sounds good to me; yes, that should make it safe :-)

All the best,
Kendy

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Jan Holesovsky-4 Jan Holesovsky-4
Reply | Threaded
Open this post in threaded view
|

Re: Hi-DPI patches for 4.2

Hi Keith,

Keith Curtis píše v St 12. 03. 2014 v 13:44 -0400:

> Who is going to do this?

I've just pushed that as

https://gerrit.libreoffice.org/8560

All the best,
Kendy

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice