Re: Some inset offset fine tuning

[racoon]

On 21.11.2016 11:16, racoon wrote:

On 20.10.2016 19:29, Jean-Marc Lasgouttes wrote:

2/ I can provide you with methods MetricsInfo:::textToInsetOffset and
MetricsInfo::textToInsetInnerOffset  that returns the pixel values
equivalent to 0.3em and 0.15em (according to your changes).

In the second case, the spacing will be larger when the font is larger,
for example in a section title. I think that this makes sense. Anyway,
the idea is to provide a method that gives a directly usable pixel
amount.


Hi JMarc,

I could make use of an em to pixel conversion for another ticket:

www.lyx.org/trac/ticket/9376#comment:9

I am using

Length(lyxrc.full_screen_width, Length::EM).inPixels(0)

I thought this gets me from em to pixel. But it seems not to give the
right result: with fixed text-width set in preferences the text still
wraps differently depending on zoom.


Maybe I misunderstood how em to pixel works. I just thought that the 
text should look the same way when zoomed just everything becomes 
larger. But I just tried a test with an html file (attached). When 
zooming in the wrapping changes as well in the browser (for some reason 
in chromes less so than in firefox but that might be explained by 
different zoom levels?). So Maybe it is all good after all. Then the 
patch is done I guess.


Daniel



	Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Re: Some inset offset fine tuning

[racoon]

On 20.10.2016 19:29, Jean-Marc Lasgouttes wrote:

2/ I can provide you with methods MetricsInfo:::textToInsetOffset and
MetricsInfo::textToInsetInnerOffset  that returns the pixel values
equivalent to 0.3em and 0.15em (according to your changes).

In the second case, the spacing will be larger when the font is larger,
for example in a section title. I think that this makes sense. Anyway,
the idea is to provide a method that gives a directly usable pixel amount.


Hi JMarc,

I could make use of an em to pixel conversion for another ticket:

www.lyx.org/trac/ticket/9376#comment:9

I am using

Length(lyxrc.full_screen_width, Length::EM).inPixels(0)

I thought this gets me from em to pixel. But it seems not to give the 
right result: with fixed text-width set in preferences the text still 
wraps differently depending on zoom.


Daniel

Re: Some inset offset fine tuning

[racoon]

On 27.10.2016 12:41, racoon wrote:

On 27.10.2016 10:21, racoon wrote:

On 27.10.2016 10:15, racoon wrote:

On 18.10.2016 09:37, Jean-Marc Lasgouttes wrote:

Le 17/10/2016 à 01:28, racoon a écrit :

I just tried to do some fine tuning on the inset offsets (minimalistic
and classic so far).


Hi racoon,

I am glad you are looking at this. I agree that small offsets are
better.


Though I have a non-high resolution display. Maybe it does not look as
good on a high-res display. If so, maybe there should be an adjustment
for high-res displays?


I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that
computes the length according to current zoom and screen resolution. I
abandonned the patch (attached) for now because it created problems for
vertical spacing. You are welcome to borrow parts of it (the margins
are
already in).

You would be welcome to have a look at it. Another option would be to
have a length in em, that is relative to the current font. That would
make the inset spacing comparable to normal spacing in the enclosing
space. This is the best solution IMO, but I am not sure that the font
information is available when we need it.


Hi JMarc,

I did have a look and the attached patch is the current result (again
only buttons and frames for now). There is also a simple test example.
Since I did not touch the vertical spacing, only inner and outer side
offsets, that should be fine still. (Though there was something wrong
with the vertical spacing to begin with since, for example, the inner
text runs over the upper and lower boundaries on higher zoom levels.)
For some reason the frames scale a bit different different and maybe
nicer than the buttons.


ps. And I did not do anything about the interactive area. So it is still
a bit off to the right and includes the outer spaces (same as in LyX
2.2.2). The translation might be possible to solve easily. I am a bit
more uncertain about the outer space.


ps2. Oh, I forgot to comment on the added pixels. They are for the
non-sizable frames. Patch commented appropriately. Though I am not fully
sure I understood all of them properly.


For some reason the scaling of the frames is nicer than the one of the 
buttons. I see this especially on very low zoom levels. I don't know why 
this is though.

Re: Some inset offset fine tuning

[racoon]

On 27.10.2016 10:21, racoon wrote:

On 27.10.2016 10:15, racoon wrote:

On 18.10.2016 09:37, Jean-Marc Lasgouttes wrote:

Le 17/10/2016 à 01:28, racoon a écrit :

I just tried to do some fine tuning on the inset offsets (minimalistic
and classic so far).


Hi racoon,

I am glad you are looking at this. I agree that small offsets are
better.


Though I have a non-high resolution display. Maybe it does not look as
good on a high-res display. If so, maybe there should be an adjustment
for high-res displays?


I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that
computes the length according to current zoom and screen resolution. I
abandonned the patch (attached) for now because it created problems for
vertical spacing. You are welcome to borrow parts of it (the margins are
already in).

You would be welcome to have a look at it. Another option would be to
have a length in em, that is relative to the current font. That would
make the inset spacing comparable to normal spacing in the enclosing
space. This is the best solution IMO, but I am not sure that the font
information is available when we need it.


Hi JMarc,

I did have a look and the attached patch is the current result (again
only buttons and frames for now). There is also a simple test example.
Since I did not touch the vertical spacing, only inner and outer side
offsets, that should be fine still. (Though there was something wrong
with the vertical spacing to begin with since, for example, the inner
text runs over the upper and lower boundaries on higher zoom levels.)
For some reason the frames scale a bit different different and maybe
nicer than the buttons.


ps. And I did not do anything about the interactive area. So it is still
a bit off to the right and includes the outer spaces (same as in LyX
2.2.2). The translation might be possible to solve easily. I am a bit
more uncertain about the outer space.


ps2. Oh, I forgot to comment on the added pixels. They are for the 
non-sizable frames. Patch commented appropriately. Though I am not fully 
sure I understood all of them properly.
From 5fc9359ece762af2792868ba6beeac278f9bde74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ram=C3=83=3F=3F=3F=3F=3F=3F=C3=83=3F=3F=3F=3F=3F?=
 =?UTF-8?q?=C3=83=3F=3F=3F=3F=C3=83=3F=3F=3F=C3=83=3F=3F=C3=83=3F=C3=82?=
 =?UTF-8?q?=C2=B6ller?= <d@web.de>
Date: Mon, 17 Oct 2016 01:21:01 +0200
Subject: [PATCH] Some inset offset fine tuning: - Make outer inset offset
 independent of inner. - Reduced the outer offset. - Make it scale with the
 zoom.

---
 src/frontends/qt4/GuiFontMetrics.cpp |  5 +++--
 src/frontends/qt4/GuiPainter.cpp |  7 ---
 src/insets/Inset.cpp | 16 
 src/insets/Inset.h   |  6 +-
 src/insets/InsetText.cpp | 15 ++-
 5 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index eade8cc..46c7dc2 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -254,7 +254,7 @@ void GuiFontMetrics::rectText(docstring const & str,
 {
static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
 
-   w = width(str) + Inset::TEXT_TO_INSET_OFFSET;
+   w = width(str) + 2 * Inset::textToInsetOffsetInside();
ascent = metrics_.ascent() + d;
descent = metrics_.descent() + d;
 }
@@ -265,7 +265,8 @@ void GuiFontMetrics::buttonText(docstring const & str,
int & w, int & ascent, int & descent) const
 {
rectText(str, w, ascent, descent);
-   w += Inset::TEXT_TO_INSET_OFFSET;
+   // 2 added pixels for the frame
+   w += Inset::textToInsetOffsetInside() + 2;
 }
 
 
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index e613f3f..1cf5575 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -593,10 +593,11 @@ void GuiPainter::buttonText(int x, int y, docstring const 
& str,
FontMetrics const & fm = theFontMetrics(font);
fm.buttonText(str, width, ascent, descent);
 
-   static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
+   static int const d = Inset::textToInsetOffsetOutside();
 
-   button(x + d, y - ascent, width - Inset::TEXT_TO_INSET_OFFSET, descent 
+ ascent, mouseHover);
-   text(x + Inset::TEXT_TO_INSET_OFFSET, y, str, font);
+   button(x + Inset::textToInsetOffsetOutside(), y - ascent, width - 
2*Inset::textToInsetOffsetOutside(), descent + ascent, mouseHover);
+   // 1 pixel added for the frame
+   text(x + Inset::textToInsetOffsetInside() + 
Inset::textToInsetOffsetOutside() + 1, y, str, font);
 }
 
 
diff --git a/src/insets/Inset.cpp b/src/insets/Inset.cpp
index 6e2e4a0..c9bf77e 100644
--- a/src/insets/Inset.cpp
+++ b/src/insets/Inset.cpp
@@ -27,6 +27,7 @@
 #include "DispatchResult.h"
 #include "FuncRequest.h"
 

Re: Some inset offset fine tuning

[racoon]

On 27.10.2016 10:15, racoon wrote:

On 18.10.2016 09:37, Jean-Marc Lasgouttes wrote:

Le 17/10/2016 à 01:28, racoon a écrit :

I just tried to do some fine tuning on the inset offsets (minimalistic
and classic so far).


Hi racoon,

I am glad you are looking at this. I agree that small offsets are better.


Though I have a non-high resolution display. Maybe it does not look as
good on a high-res display. If so, maybe there should be an adjustment
for high-res displays?


I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that
computes the length according to current zoom and screen resolution. I
abandonned the patch (attached) for now because it created problems for
vertical spacing. You are welcome to borrow parts of it (the margins are
already in).

You would be welcome to have a look at it. Another option would be to
have a length in em, that is relative to the current font. That would
make the inset spacing comparable to normal spacing in the enclosing
space. This is the best solution IMO, but I am not sure that the font
information is available when we need it.


Hi JMarc,

I did have a look and the attached patch is the current result (again
only buttons and frames for now). There is also a simple test example.
Since I did not touch the vertical spacing, only inner and outer side
offsets, that should be fine still. (Though there was something wrong
with the vertical spacing to begin with since, for example, the inner
text runs over the upper and lower boundaries on higher zoom levels.)
For some reason the frames scale a bit different different and maybe
nicer than the buttons.


ps. And I did not do anything about the interactive area. So it is still 
a bit off to the right and includes the outer spaces (same as in LyX 
2.2.2). The translation might be possible to solve easily. I am a bit 
more uncertain about the outer space.

Re: Some inset offset fine tuning

[racoon]

On 18.10.2016 09:37, Jean-Marc Lasgouttes wrote:

Le 17/10/2016 à 01:28, racoon a écrit :

I just tried to do some fine tuning on the inset offsets (minimalistic
and classic so far).


Hi racoon,

I am glad you are looking at this. I agree that small offsets are better.


Though I have a non-high resolution display. Maybe it does not look as
good on a high-res display. If so, maybe there should be an adjustment
for high-res displays?


I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that
computes the length according to current zoom and screen resolution. I
abandonned the patch (attached) for now because it created problems for
vertical spacing. You are welcome to borrow parts of it (the margins are
already in).

You would be welcome to have a look at it. Another option would be to
have a length in em, that is relative to the current font. That would
make the inset spacing comparable to normal spacing in the enclosing
space. This is the best solution IMO, but I am not sure that the font
information is available when we need it.


Hi JMarc,

I did have a look and the attached patch is the current result (again 
only buttons and frames for now). There is also a simple test example. 
Since I did not touch the vertical spacing, only inner and outer side 
offsets, that should be fine still. (Though there was something wrong 
with the vertical spacing to begin with since, for example, the inner 
text runs over the upper and lower boundaries on higher zoom levels.) 
For some reason the frames scale a bit different different and maybe 
nicer than the buttons.


Daniel
From 313b32c0ca5b5619010169097aca6a9a29511276 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ram=C3=83=3F=3F=3F=3F=3F=C3=83=3F=3F=3F=3F=C3=83?=
 =?UTF-8?q?=3F=3F=3F=C3=83=3F=3F=C3=83=3F=C3=82=C2=B6ller?= <d@web.de>
Date: Mon, 17 Oct 2016 01:21:01 +0200
Subject: [PATCH] Some inset offset fine tuning: - Make outer inset offset
 independent of inner. - Reduced the outer offset. - Make it scale with the
 zoom.

---
 src/frontends/qt4/GuiFontMetrics.cpp |  4 ++--
 src/frontends/qt4/GuiPainter.cpp |  6 +++---
 src/insets/Inset.cpp | 16 
 src/insets/Inset.h   |  6 +-
 src/insets/InsetText.cpp | 13 -
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index eade8cc..2ed3db4 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -254,7 +254,7 @@ void GuiFontMetrics::rectText(docstring const & str,
 {
static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
 
-   w = width(str) + Inset::TEXT_TO_INSET_OFFSET;
+   w = Inset::textToInsetOffsetInside() + width(str) + 
Inset::textToInsetOffsetInside();
ascent = metrics_.ascent() + d;
descent = metrics_.descent() + d;
 }
@@ -265,7 +265,7 @@ void GuiFontMetrics::buttonText(docstring const & str,
int & w, int & ascent, int & descent) const
 {
rectText(str, w, ascent, descent);
-   w += Inset::TEXT_TO_INSET_OFFSET;
+   w += Inset::textToInsetOffsetInside() + 2;
 }
 
 
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index e613f3f..10d3dfa 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -593,10 +593,10 @@ void GuiPainter::buttonText(int x, int y, docstring const 
& str,
FontMetrics const & fm = theFontMetrics(font);
fm.buttonText(str, width, ascent, descent);
 
-   static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
+   static int const d = Inset::textToInsetOffsetOutside();
 
-   button(x + d, y - ascent, width - Inset::TEXT_TO_INSET_OFFSET, descent 
+ ascent, mouseHover);
-   text(x + Inset::TEXT_TO_INSET_OFFSET, y, str, font);
+   button(x + Inset::textToInsetOffsetOutside(), y - ascent, width - 
2*Inset::textToInsetOffsetOutside(), descent + ascent, mouseHover);
+   text(x + Inset::textToInsetOffsetInside() + 
Inset::textToInsetOffsetOutside() + 1, y, str, font);
 }
 
 
diff --git a/src/insets/Inset.cpp b/src/insets/Inset.cpp
index 6e2e4a0..c9bf77e 100644
--- a/src/insets/Inset.cpp
+++ b/src/insets/Inset.cpp
@@ -27,6 +27,7 @@
 #include "DispatchResult.h"
 #include "FuncRequest.h"
 #include "FuncStatus.h"
+#include "Length.h"
 #include "MetricsInfo.h"
 #include "output_xhtml.h"
 #include "Text.h"
@@ -621,6 +622,21 @@ ColorCode Inset::labelColor() const
 }
 
 
+int Inset::textToInsetOffsetInside()
+{
+   // 2 pixel at 100dpi default resolution is
+   // equivalent to .5 millimeter.
+   return Length(.5, Length::MM).inPixels(0);
+}
+
+int Inset::textToInsetOffsetOutside()
+{
+   // 1 pixel at 100dpi default resolution is
+   // equivalent to .25 millimeter.
+   return Length(.

Re: Some inset offset fine tuning

[Jean-Marc Lasgouttes]

Le 22/10/2016 à 11:33, racoon a écrit :

Does anyone happen to know where the area of insets for clicks and hover
with the mouse is set? I don't mean the paintings but the interaction.
There seem to be several problems with it I could have a look at.

1. The area if off by a couple of pixels, i.e. translated to the right.
2. The area includes the spaces around the insets.


Normally, the area is the whole dimension of the inset, that is 
everything that is added in metrics() method. For layout and mouse 
purpose, an inset is defined by its (x, y) position and its dimension.


What you describe can happen when metrics() computes in one way, but 
draw() uses different logic to compute offsets.


JMarc

Re: Some inset offset fine tuning

[racoon]

On 21.10.2016 16:29, Jean-Marc Lasgouttes wrote:

Le 21/10/2016 à 16:18, racoon a écrit :

Just an update of the fix. No work done on the scaling yet or other
insets yet. But I realised that the cursor takes the space of the first
pixels of the character. So the space between insets has at least to be
two rather than one pixel wide. I have also attached my test file. I
have tested an measured now without anti-aliasing.


A question: I still see some 1 pixel explicit offset. What is it?
Cursor? Note that cursor width can be tuned in the preferences.

Even if scaling is not  in scope yet, it would be nice to understand
what each pixel value is and how it should scale. For example, I tend to
think that al borders (default width) should have a scalable width.
Maybe cursor too.


Does anyone happen to know where the area of insets for clicks and hover 
with the mouse is set? I don't mean the paintings but the interaction. 
There seem to be several problems with it I could have a look at.


1. The area if off by a couple of pixels, i.e. translated to the right.
2. The area includes the spaces around the insets.

Daniel

Re: Some inset offset fine tuning

[racoon]

On 21.10.2016 16:29, Jean-Marc Lasgouttes wrote:

Le 21/10/2016 à 16:18, racoon a écrit :

Just an update of the fix. No work done on the scaling yet or other
insets yet. But I realised that the cursor takes the space of the first
pixels of the character. So the space between insets has at least to be
two rather than one pixel wide. I have also attached my test file. I
have tested an measured now without anti-aliasing.


A question: I still see some 1 pixel explicit offset. What is it?
Cursor? Note that cursor width can be tuned in the preferences.


No it is not the cursor with. The cursor just takes the space of the 
first pixels of the character.


Here are some independent considerations.

One choice that has to be made is whether the insets should be treated 
exactly like characters, i.e. the cursor starts directly at the edge 
(not before) the inset when you put the cursor at the inset so that 
typing would insert characters before the inset. If this is done then 
the cursor is hard to see when the frame of an inset is darkish. That is 
why I opted for putting it one pixel before the inset. But if the insets 
should be treated as characters this seems not exactly right. So 
alternatively, one could go for an inverted cursor rather than a black one.



Even if scaling is not  in scope yet, it would be nice to understand
what each pixel value is and how it should scale. For example, I tend to
think that al borders (default width) should have a scalable width.
Maybe cursor too.


Yes, I don't understand the connections extra pixels yet. But I'll make 
sure to either comment in the source when I do or replace them with 
proper variables.


Daniel

Re: Some inset offset fine tuning

[Jean-Marc Lasgouttes]

Le 21/10/2016 à 16:18, racoon a écrit :

Just an update of the fix. No work done on the scaling yet or other
insets yet. But I realised that the cursor takes the space of the first
pixels of the character. So the space between insets has at least to be
two rather than one pixel wide. I have also attached my test file. I
have tested an measured now without anti-aliasing.


A question: I still see some 1 pixel explicit offset. What is it? 
Cursor? Note that cursor width can be tuned in the preferences.


Even if scaling is not  in scope yet, it would be nice to understand 
what each pixel value is and how it should scale. For example, I tend to 
think that al borders (default width) should have a scalable width. 
Maybe cursor too.


JMarc

Re: Some inset offset fine tuning

[racoon]

Just an update of the fix. No work done on the scaling yet or other 
insets yet. But I realised that the cursor takes the space of the first 
pixels of the character. So the space between insets has at least to be 
two rather than one pixel wide. I have also attached my test file. I 
have tested an measured now without anti-aliasing.
From 06e40ada85c13ea22c4ea484f338fdf7cefef57e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ram=C3=83=3F=C3=82=C2=B6ller?= <d@web.de>
Date: Mon, 17 Oct 2016 01:21:01 +0200
Subject: [PATCH] Some inset offset fine tuning: - Make outer inset offset
 independent of inner. - Reduced the outer offset.

---
 src/frontends/qt4/GuiFontMetrics.cpp |  4 ++--
 src/frontends/qt4/GuiPainter.cpp |  6 +++---
 src/insets/Inset.h   |  6 +-
 src/insets/InsetText.cpp | 13 -
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index eade8cc..43cf0d1 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -254,7 +254,7 @@ void GuiFontMetrics::rectText(docstring const & str,
 {
static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
 
-   w = width(str) + Inset::TEXT_TO_INSET_OFFSET;
+   w = Inset::TEXT_TO_INSET_OFFSET_INSIDE + width(str) + 
Inset::TEXT_TO_INSET_OFFSET_INSIDE;
ascent = metrics_.ascent() + d;
descent = metrics_.descent() + d;
 }
@@ -265,7 +265,7 @@ void GuiFontMetrics::buttonText(docstring const & str,
int & w, int & ascent, int & descent) const
 {
rectText(str, w, ascent, descent);
-   w += Inset::TEXT_TO_INSET_OFFSET;
+   w += 2 * Inset::TEXT_TO_INSET_OFFSET_OUTSIDE;
 }
 
 
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index e613f3f..1774939 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -593,10 +593,10 @@ void GuiPainter::buttonText(int x, int y, docstring const 
& str,
FontMetrics const & fm = theFontMetrics(font);
fm.buttonText(str, width, ascent, descent);
 
-   static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
+   static int const d = Inset::TEXT_TO_INSET_OFFSET_OUTSIDE;
 
-   button(x + d, y - ascent, width - Inset::TEXT_TO_INSET_OFFSET, descent 
+ ascent, mouseHover);
-   text(x + Inset::TEXT_TO_INSET_OFFSET, y, str, font);
+   button(x + d, y - ascent, width - 2 * d, descent + ascent, mouseHover);
+   text(x + Inset::TEXT_TO_INSET_OFFSET_INSIDE + 
Inset::TEXT_TO_INSET_OFFSET_OUTSIDE, y, str, font);
 }
 
 
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index 48ad7be..34ba4e9 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -574,7 +574,11 @@ public:
///
virtual ColorCode labelColor() const;
//
-   enum { TEXT_TO_INSET_OFFSET = 4 };
+   enum {
+   TEXT_TO_INSET_OFFSET = 4,
+   TEXT_TO_INSET_OFFSET_INSIDE = 2,
+   TEXT_TO_INSET_OFFSET_OUTSIDE = 1
+   };
 
 protected:
/// Constructors
diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index df4f6e2..e3a18f1 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -208,7 +208,8 @@ void InsetText::metrics(MetricsInfo & mi, Dimension & dim) 
const
mi.base.textwidth += 2 * TEXT_TO_INSET_OFFSET;
dim.asc += TEXT_TO_INSET_OFFSET;
dim.des += TEXT_TO_INSET_OFFSET;
-   dim.wid += 2 * TEXT_TO_INSET_OFFSET;
+   // cursor position after inset
+   dim.wid += 2 * (TEXT_TO_INSET_OFFSET_INSIDE + 
TEXT_TO_INSET_OFFSET_OUTSIDE + 1);
 }
 
 
@@ -216,10 +217,10 @@ void InsetText::draw(PainterInfo & pi, int x, int y) const
 {
TextMetrics & tm = pi.base.bv->textMetrics(_);
 
-   int const w = tm.width() + TEXT_TO_INSET_OFFSET;
+   int const w = tm.width() + 2 * TEXT_TO_INSET_OFFSET_INSIDE + 1;
int const yframe = y - TEXT_TO_INSET_OFFSET - tm.ascent();
int const h = tm.height() + 2 * TEXT_TO_INSET_OFFSET;
-   int const xframe = x + TEXT_TO_INSET_OFFSET / 2;
+   int const xframe = x + TEXT_TO_INSET_OFFSET_OUTSIDE;
bool change_drawn = false;
if (drawFrame_ || pi.full_repaint) {
if (pi.full_repaint)
@@ -251,7 +252,8 @@ void InsetText::draw(PainterInfo & pi, int x, int y) const
pi.backgroundColor(this, false));
// The change tracking cue must not be inherited
Changer dummy2 = make_change(pi.change_, Change());
-   tm.draw(pi, x + TEXT_TO_INSET_OFFSET, y);
+   // text position within inset
+   tm.draw(pi, x + TEXT_TO_INSET_OFFSET_INSIDE + 
TEXT_TO_INSET_OFFSET_OUTSIDE + 1, y);
}
if (canPaintChange(*pi.base.bv) && (!change_drawn || 
pi.change_.deleted()))
// D

Re: Some inset offset fine tuning

[Jean-Marc Lasgouttes]

Le 18/10/2016 à 12:11, racoon a écrit :

Thanks JMarc. Do I understand you correctly that you tested high res
displays and they actually create a problem with those tiny 1px spaces?


One other user tested on a HiDPI display and found that the spaces 
(which use several pixels on those displays) would increase the 
interline spacing. Therefore, TEXT_TO_INSET_OFFSET should not be used 
vertically, or at least the value should be capped to avoid increasing 
interline. Nothing terribly difficult, I guess, but it needs to be done 
carefully..



Since I can't test on a high res display: would a zoom of, say, 100 %
result in the same height in pixels of a given font on high res and low
res displays?


No. On a high res display, in order to have a given text still legible, 
the height of the font in pixel is more important.


To understand what happens, you should set zoom to 300% and step back a 
few meters from your monitor :)


We have two strategies:
1/ like in my patch, we can define methods Inset::textToInsetOffset and 
Inset::textToInsetInnerOffset maybe with shorter names :) that returns 
the pixel values equivalent to 1mm and 0.5mm (according to your changes).
2/ I can provide you with methods MetricsInfo:::textToInsetOffset and 
MetricsInfo::textToInsetInnerOffset  that returns the pixel values 
equivalent to 0.3em and 0.15em (according to your changes).


In the second case, the spacing will be larger when the font is larger, 
for example in a section title. I think that this makes sense. Anyway, 
the idea is to provide a method that gives a directly usable pixel amount.


Why these magic numbers, I hear you (almost) say? See the rambling below.

I suspect that I have lost you already, but I also wrote this down for 
my own benefit.


The short term plan could be to do your work of using the right constant 
at the right place. Then we have to use something like my patch that 
makes sure that the spacing looks good on HiDPI screens too.


Sorry for the complicated explanations :)

JMarc

==
Appendix: some computations.
==

The idea of my patch is that 1 pixel on a old-school 100dpi monitor at 
100% zoom is
1/100 inch (dpi means dots per inch), that is 0,254 millimeters. So the 
default TEXT_TO_INSET_OFFSET value of 4 pixels is actually 1mm. Then we 
can do our computations in millimeters and have LyX use the right pixel 
spacing. The class Length can do this for us.


Now that I think of it, I would say that the inset spacing should be 
proportional to the normal text spacing in the current font. This 
information is generally available at places where one wants to compute 
spacing, AFAIK. The right unit for having length that depend from 
current font is 'em' (or 'mu' if you are in math formulas).


By default, the normal font has a point size of 10 (look in 
preferences). A point size, if I understand correctly 
(https://en.wikipedia.org/wiki/Point_(typography)#Desktop_publishing_point) 
is 1/72 of inch.


Therefore, at 100dpi, a pixel is 4/3 of point. Therefore a 10 points 
font would have a pixel size of 40/3 =13.333... This pixel size is 
actually what one calls a em. We can get this value using the em() 
method of the FontMetrics class.


All this computation to say that, with this second scheme,  the value of 
one pixel on a normal font size on a normal monitor would be 3/40em and 
therefore the value TEXT_TO_INSET_OFFSET of 4 pixels is now equivalent 
to 0.3em.

Re: Some inset offset fine tuning

[racoon]

On 18.10.2016 09:37, Jean-Marc Lasgouttes wrote:

I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that
computes the length according to current zoom and screen resolution. I
abandonned the patch (attached) for now because it created problems for
vertical spacing.


Yes, maybe the differentiations between the horizontal spacing and 
vertical that I did could be helpful there. I'll take a look.


Daniel

Re: Some inset offset fine tuning

[racoon]

On 18.10.2016 09:37, Jean-Marc Lasgouttes wrote:

Le 17/10/2016 à 01:28, racoon a écrit :

Though I have a non-high resolution display. Maybe it does not look as
good on a high-res display. If so, maybe there should be an adjustment
for high-res displays?


I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that
computes the length according to current zoom and screen resolution. I
abandonned the patch (attached) for now because it created problems for
vertical spacing. You are welcome to borrow parts of it (the margins are
already in).


Thanks JMarc. Do I understand you correctly that you tested high res 
displays and they actually create a problem with those tiny 1px spaces?



You would be welcome to have a look at it. Another option would be to
have a length in em, that is relative to the current font. That would
make the inset spacing comparable to normal spacing in the enclosing
space. This is the best solution IMO, but I am not sure that the font
information is available when we need it.


Since I can't test on a high res display: would a zoom of, say, 100 % 
result in the same height in pixels of a given font on high res and low 
res displays?


Daniel

Re: Some inset offset fine tuning

[Jean-Marc Lasgouttes]

Le 17/10/2016 à 01:28, racoon a écrit :

I just tried to do some fine tuning on the inset offsets (minimalistic
and classic so far).


Hi racoon,

I am glad you are looking at this. I agree that small offsets are better.


Though I have a non-high resolution display. Maybe it does not look as
good on a high-res display. If so, maybe there should be an adjustment
for high-res displays?


I had a go earlier to replace TEXT_TO_INSET_OFFSET with a function that 
computes the length according to current zoom and screen resolution. I 
abandonned the patch (attached) for now because it created problems for 
vertical spacing. You are welcome to borrow parts of it (the margins are 
already in).


You would be welcome to have a look at it. Another option would be to 
have a length in em, that is relative to the current font. That would 
make the inset spacing comparable to normal spacing in the enclosing 
space. This is the best solution IMO, but I am not sure that the font 
information is available when we need it.


JMarc

From a6c9f8a782d561ccea0bd5c2bb70c71527280c0e Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes 
Date: Thu, 12 Nov 2015 10:19:08 +0100
Subject: [PATCH] Replace hardcoded TEXT_TO_INSET_OFFSET by dynamic value

The value is counted as 1 millimeter now (the same at 100 dpi). We also ensures 
that it measures at least 1 pixel.

Similarly, the Workarea default margin is now 2.5mm instead of being hardcoded 
to 10.

This should improve the situation for HiDPI systems.
---
 src/BufferView.cpp   |  6 --
 src/RowPainter.cpp   |  2 +-
 src/frontends/qt4/GuiFontMetrics.cpp |  6 +++---
 src/frontends/qt4/GuiPainter.cpp |  4 ++--
 src/insets/Inset.cpp |  9 +
 src/insets/Inset.h   |  2 +-
 src/insets/InsetCaption.cpp  |  8 
 src/insets/InsetCollapsable.cpp  |  6 +++---
 src/insets/InsetIPA.cpp  | 12 ++--
 src/insets/InsetPhantom.cpp  |  4 ++--
 src/insets/InsetPreview.cpp  | 12 ++--
 src/insets/InsetTabular.cpp  |  4 ++--
 src/insets/InsetText.cpp | 28 ++--
 src/insets/RenderGraphic.cpp | 18 +-
 src/insets/RenderPreview.cpp |  2 +-
 15 files changed, 67 insertions(+), 56 deletions(-)

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 0030f33..902f8a7 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -351,11 +351,13 @@ BufferView::~BufferView()
 
 int BufferView::rightMargin() const
 {
+   // THe value used to be hardcoded to 10, which is 2.5mm at 100dpi
+   int const default_margin = Length(2.5, Length::MM).inPixels(0);
// The additional test for the case the outliner is opened.
if (!full_screen_ ||
!lyxrc.full_screen_limit ||
-   width_ < lyxrc.full_screen_width + 20)
-   return 10;
+   width_ < lyxrc.full_screen_width + 2 * default_margin)
+   return default_margin;
 
return (width_ - lyxrc.full_screen_width) / 2;
 }
diff --git a/src/RowPainter.cpp b/src/RowPainter.cpp
index ce5780b..3d360d4 100644
--- a/src/RowPainter.cpp
+++ b/src/RowPainter.cpp
@@ -540,7 +540,7 @@ void RowPainter::paintLast()
FontMetrics const & fm = theFontMetrics(font);
int const size = int(0.75 * fm.maxAscent());
int const y = yo_ - size;
-   int const max_row_width = width_ - size - 
Inset::TEXT_TO_INSET_OFFSET;
+   int const max_row_width = width_ - size - 
Inset::textToInsetOffset();
int x = is_rtl ? nestMargin() + changebarMargin()
: max_row_width - row_.right_margin;
 
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index 8d0b026..d4f3585 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -237,9 +237,9 @@ bool GuiFontMetrics::breakAt(docstring & s, int & x, bool 
const rtl, bool const
 void GuiFontMetrics::rectText(docstring const & str,
int & w, int & ascent, int & descent) const
 {
-   static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
+   static int const d = Inset::textToInsetOffset() / 2;
 
-   w = width(str) + Inset::TEXT_TO_INSET_OFFSET;
+   w = width(str) + Inset::textToInsetOffset();
ascent = metrics_.ascent() + d;
descent = metrics_.descent() + d;
 }
@@ -250,7 +250,7 @@ void GuiFontMetrics::buttonText(docstring const & str,
int & w, int & ascent, int & descent) const
 {
rectText(str, w, ascent, descent);
-   w += Inset::TEXT_TO_INSET_OFFSET;
+   w += Inset::textToInsetOffset();
 }
 
 
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index 2b024d8..af75547 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -533,10 +533,10 @@ 

Re: Some inset offset fine tuning

[racoon]

On 17.10.2016 01:28, racoon wrote:

I just tried to do some fine tuning on the inset offsets (minimalistic
and classic so far).

[...]


Because my message did not go through to the list at first, I posted it 
on trac:


http://www.lyx.org/trac/ticket/10441

Daniel

Some inset offset fine tuning

[racoon]

I just tried to do some fine tuning on the inset offsets (minimalistic 
and classic so far).


1. I have decoupled the outer and inner offset at the beginning and at 
the end of the inset.


2. I have changed the outer offset to 1px.

I like the smaller spaces better. Now they cannot be mistaken for spaces 
anymore even with small font-sizes.


Though I have a non-high resolution display. Maybe it does not look as 
good on a high-res display. If so, maybe there should be an adjustment 
for high-res displays?


Daniel
From 93f644687e9d5fc161b24eb89dc35e1095278633 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ram=C3=B6ller?= <d@web.de>
Date: Mon, 17 Oct 2016 01:21:01 +0200
Subject: [PATCH] Some inset offset fine tuning: - Make outer inset offset
 independent of inner. - Reduced the outer offset.

---
 src/frontends/qt4/GuiFontMetrics.cpp |  4 ++--
 src/frontends/qt4/GuiPainter.cpp |  6 +++---
 src/insets/Inset.h   |  6 +-
 src/insets/InsetText.cpp | 13 -
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index eade8cc..29f8f3b 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -254,7 +254,7 @@ void GuiFontMetrics::rectText(docstring const & str,
 {
static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
 
-   w = width(str) + Inset::TEXT_TO_INSET_OFFSET;
+   w = width(str) + Inset::TEXT_TO_INSET_OFFSET_INSIDE + 
Inset::TEXT_TO_INSET_OFFSET_INSIDE;
ascent = metrics_.ascent() + d;
descent = metrics_.descent() + d;
 }
@@ -265,7 +265,7 @@ void GuiFontMetrics::buttonText(docstring const & str,
int & w, int & ascent, int & descent) const
 {
rectText(str, w, ascent, descent);
-   w += Inset::TEXT_TO_INSET_OFFSET;
+   w += Inset::TEXT_TO_INSET_OFFSET_INSIDE;
 }
 
 
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index e613f3f..23d4e73 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -593,10 +593,10 @@ void GuiPainter::buttonText(int x, int y, docstring const 
& str,
FontMetrics const & fm = theFontMetrics(font);
fm.buttonText(str, width, ascent, descent);
 
-   static int const d = Inset::TEXT_TO_INSET_OFFSET / 2;
+   static int const d = Inset::TEXT_TO_INSET_OFFSET_OUTSIDE;
 
-   button(x + d, y - ascent, width - Inset::TEXT_TO_INSET_OFFSET, descent 
+ ascent, mouseHover);
-   text(x + Inset::TEXT_TO_INSET_OFFSET, y, str, font);
+   button(x + d, y - ascent, width - Inset::TEXT_TO_INSET_OFFSET_OUTSIDE, 
descent + ascent, mouseHover);
+   text(x + Inset::TEXT_TO_INSET_OFFSET_OUTSIDE + 
Inset::TEXT_TO_INSET_OFFSET_INSIDE, y, str, font);
 }
 
 
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index 48ad7be..34ba4e9 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -574,7 +574,11 @@ public:
///
virtual ColorCode labelColor() const;
//
-   enum { TEXT_TO_INSET_OFFSET = 4 };
+   enum {
+   TEXT_TO_INSET_OFFSET = 4,
+   TEXT_TO_INSET_OFFSET_INSIDE = 2,
+   TEXT_TO_INSET_OFFSET_OUTSIDE = 1
+   };
 
 protected:
/// Constructors
diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index df4f6e2..e30d07a 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -208,7 +208,8 @@ void InsetText::metrics(MetricsInfo & mi, Dimension & dim) 
const
mi.base.textwidth += 2 * TEXT_TO_INSET_OFFSET;
dim.asc += TEXT_TO_INSET_OFFSET;
dim.des += TEXT_TO_INSET_OFFSET;
-   dim.wid += 2 * TEXT_TO_INSET_OFFSET;
+   // cursor position after inset
+   dim.wid += 2 * (TEXT_TO_INSET_OFFSET_INSIDE + 
TEXT_TO_INSET_OFFSET_OUTSIDE);
 }
 
 
@@ -216,10 +217,10 @@ void InsetText::draw(PainterInfo & pi, int x, int y) const
 {
TextMetrics & tm = pi.base.bv->textMetrics(_);
 
-   int const w = tm.width() + TEXT_TO_INSET_OFFSET;
+   int const w = tm.width() + 2 * TEXT_TO_INSET_OFFSET_INSIDE;
int const yframe = y - TEXT_TO_INSET_OFFSET - tm.ascent();
int const h = tm.height() + 2 * TEXT_TO_INSET_OFFSET;
-   int const xframe = x + TEXT_TO_INSET_OFFSET / 2;
+   int const xframe = x + TEXT_TO_INSET_OFFSET_OUTSIDE;
bool change_drawn = false;
if (drawFrame_ || pi.full_repaint) {
if (pi.full_repaint)
@@ -251,7 +252,8 @@ void InsetText::draw(PainterInfo & pi, int x, int y) const
pi.backgroundColor(this, false));
// The change tracking cue must not be inherited
Changer dummy2 = make_change(pi.change_, Change());
-   tm.draw(pi, x + TEXT_TO_INSET_OFFSET, y);
+   // text position within inset
+   tm.draw(pi, x + TEXT_