On Thu, Jan 19, 2017 at 04:04:15PM +0100, Jean-Marc Lasgouttes wrote:
> Le 18/01/2017 à 19:23, Enrico Forestieri a écrit :
> > > I am not completely sure what this styleName thing does. Is it mac/windows
> > > only?
> >
> > No, it is platform independent. For example, if you ask for a font with
> > name cmsy10, Qt would pick the first cmsy10.ttf it stumbles upon. This
> > would be unfortunate because, due to its idiosyncratic behavior, Qt will
> > not display many symbols with a certain codepoint. So, we replicated some
> > glyphs at different positions and tell Qt to display those ones. In order
> > to be sure of using our fonts, we ask for a font with a given name *and*
> > style "LyX". This is our own style name, so we can be sure we are using
> > the right fonts.
>
> I was asking because of the sentence in the QFont docs:
>
> It depends on system font support, thus only works for macOS and X11 so
> far. On Windows
> irregular styles will be added as separate font families so there is no
> need for this.
Exactly. On Windows private fonts registered with addApplicationFont are
always preferred over the installed ones, but not when using fontconfig.
So, this is needed on macOS and X11 but does not hurt on Windows.
> > > Is it Qt 4.8+ only?
> >
> > Yes, that method was introduced in Qt 4.8.
>
> Does it mean that the bug does not exist on earlier systems, or that our
> workaround does not work?
Which workaround? When using earlier versions of Qt, many symbols might
nevertheless be broken, but also checking for s.length() == 1 makes \notin
and \neq still work.
> > > Here is what I came up with. Is it what you had in mind, or should the ||
> > > in
> > > the test become a && ?
> >
> > No, the || is fine. My rationale is the following. Qt has its own idea
> > of what a font should contain, and in the past we had to workaround its
> > shortcomings several times. So, when we are sure that a certain code works
> > with our fonts, we should stick with it to avoid bad surprises. Now we have
> > a way of telling when we are dealing with our fonts through the styleName
> > thing.
> >
> > > You can see that the \kern in the definition of \not is gone, and this is
> > > definitely a good thing.
> >
> > However, the issue does not manifest in master, meaning
> > that someone deployed a corrective action to avoid it. We have to
> > understand what this action was and then decide.
>
> Here is what happened
> 1/ I change randomly (!) the spacing of math equations in the MathRow work
> 2/ as a consequence the formulas in lib/symbols are often wrong
> 3/ I fix the problem with arabic test at e832d2e90f
> 4/ I do a new round of modifications of lib/symbols at 7335ee8e, without
> realizing that what I am fixing is a consequence of the commit 3/ and not of
> the ongoing work 1/
>
> Does this make sense to you? The fact that the right value now is without
> explicit kern is a proof that all this work on math formula spacing has been
> done correctly (almost!)
Yes, it makes sense. I did not follow your recent work on math spacing, but
after looking at the sources in master I realized that you was responsible
for the fact that now \not works properly in master :)
I also understood why the \kern in the definition of \not is not needed
anymore after fixing the problem with zero-width characters. I was also
able to replicate it in stable (see the attached not1.diff).
I think that \not was tailored to work properly only with "=" and "\in"
(in general, with symbols marked as mathrel) as it fails to display
properly with other characters (try "\not a" in stable). Instead, it
now works in master with anything.
It is not possible to fix it in stable without backporting your work.
The best I was able to come up with is the second patch not2.diff.
In this case, "\not a", \neq, and \notin work, but \not= and \not\in
are broken.
In conclusion, your patch textwidth2.diff is the right thing to do
in master. For stable we have to make a decision. The alternatives are:
1) The patch I posted earlier (textwidth.diff), amended to account
for single chars. This restores the previous behavior, where \not
only works properly with mathrel operators.
2) The not1.diff patch. This is essentially equivalent to 1) but avoids
the \kern in the definition of \not.
3) The not2.diff patch. This makes \ne, \neq, and \notin work again
and fixes the usage of \not with other non-mathrel characters,
while breaking it for \not=, \not\in and other mathrel operators.
I would suggest to choose either 1) or 2), which simply restore the
previous behavior.
--
Enrico
diff --git a/lib/symbols b/lib/symbols
index 5dc3e4c..99c9e77 100644
--- a/lib/symbols
+++ b/lib/symbols
@@ -291,8 +291,7 @@ spadesuit cmsy 127 170 mathord ♠
# We define lyxnot as mathrel in order to have proper alignment
lyxnot cmsy 54 47 mathrel /
iffont cmsy
-# 9mu = 0.5em which is the extra space added to relation operators
-\def\not{\lyxnot\kern-9mu}
+\def\not{\lyxnot}
else
\def\not{\kern4mu\lyxnot\kern-19mu}
endif
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp
b/src/frontends/qt4/GuiFontMetrics.cpp
index f17ac37..0eb29e4 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -181,13 +181,22 @@ int GuiFontMetrics::width(docstring const & s) const
// For some reason QMetrics::width returns a wrong value with Qt5
// int w = metrics_.width(toqstr(s));
#endif
- QTextLayout tl;
- tl.setText(toqstr(s));
- tl.setFont(font_);
- tl.beginLayout();
- QTextLine line = tl.createLine();
- tl.endLayout();
- int w = int(line.naturalTextWidth());
+ int w;
+ if (s.length() == 1
+#if QT_VERSION >= 0x040800
+ || font_.styleName() == "LyX"
+#endif
+ )
+ w = metrics_.width(toqstr(s));
+ else {
+ QTextLayout tl;
+ tl.setText(toqstr(s));
+ tl.setFont(font_);
+ tl.beginLayout();
+ QTextLine line = tl.createLine();
+ tl.endLayout();
+ w = int(line.naturalTextWidth());
+ }
#ifdef CACHE_METRICS_WIDTH
strwidth_cache_.insert(s, new int(w), s.size() * sizeof(char_type));
#endif
diff --git a/src/mathed/InsetMathSymbol.cpp b/src/mathed/InsetMathSymbol.cpp
index a160dec..e5eda44 100644
--- a/src/mathed/InsetMathSymbol.cpp
+++ b/src/mathed/InsetMathSymbol.cpp
@@ -80,10 +80,12 @@ void InsetMathSymbol::metrics(MetricsInfo & mi, Dimension &
dim) const
dim.des -= h_;
}
// seperate things a bit
- if (isRelOp())
- dim.wid += support::iround(0.5 * em);
- else
- dim.wid += support::iround(0.1667 * em);
+ if (sym_->name != "lyxnot") {
+ if (isRelOp())
+ dim.wid += support::iround(0.5 * em);
+ else
+ dim.wid += support::iround(0.1667 * em);
+ }
scriptable_ = false;
if (mi.base.style == LM_ST_DISPLAY)
diff --git a/lib/symbols b/lib/symbols
index 5dc3e4c..e571ced 100644
--- a/lib/symbols
+++ b/lib/symbols
@@ -292,7 +292,7 @@ spadesuit cmsy 127 170 mathord ♠
lyxnot cmsy 54 47 mathrel /
iffont cmsy
# 9mu = 0.5em which is the extra space added to relation operators
-\def\not{\lyxnot\kern-9mu}
+\def\not{\lyxnot}
else
\def\not{\kern4mu\lyxnot\kern-19mu}
endif
@@ -1123,9 +1123,9 @@ pod lyxblacktext 0 0 func x
amsmath
# pre-defined macros
#
-\def\neq{\not=} mathrel ≠
-\def\ne{\not=} mathrel ≠
-\def\notin{\not\in} mathrel ∉
+\def\neq{\not\kern-9mu=} mathrel ≠
+\def\ne{\not\kern-9mu=} mathrel ≠
+\def\notin{\not\kern-9mu\in} mathrel ∉
\def\slash{/}
\def\longleftrightarrow{\leftarrow\kern-12.5mu\rightarrow}
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp
b/src/frontends/qt4/GuiFontMetrics.cpp
index f17ac37..0eb29e4 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -181,13 +181,22 @@ int GuiFontMetrics::width(docstring const & s) const
// For some reason QMetrics::width returns a wrong value with Qt5
// int w = metrics_.width(toqstr(s));
#endif
- QTextLayout tl;
- tl.setText(toqstr(s));
- tl.setFont(font_);
- tl.beginLayout();
- QTextLine line = tl.createLine();
- tl.endLayout();
- int w = int(line.naturalTextWidth());
+ int w;
+ if (s.length() == 1
+#if QT_VERSION >= 0x040800
+ || font_.styleName() == "LyX"
+#endif
+ )
+ w = metrics_.width(toqstr(s));
+ else {
+ QTextLayout tl;
+ tl.setText(toqstr(s));
+ tl.setFont(font_);
+ tl.beginLayout();
+ QTextLine line = tl.createLine();
+ tl.endLayout();
+ w = int(line.naturalTextWidth());
+ }
#ifdef CACHE_METRICS_WIDTH
strwidth_cache_.insert(s, new int(w), s.size() * sizeof(char_type));
#endif
