Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
Le 19/01/2017 à 16:04, Jean-Marc Lasgouttes a écrit : 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!) Makes sense to me. Lots of changes to spacing, and lib/symbols that lags behind. A bug slips by the new features. Nice that this odd kern is not needed after all. The value has been found by trial-and-error. However may I suggest: what's good for display is not necessarily the best for structural interaction—can we reintroduce a kern to make it slightly imperfect, so that there is feedback during selection (as you did in current master for \Arrownot)? Another thing I would have mentioned is indeed that \{A,a}rrownot works on the same principle, but I saw that you already took care of it. (speaking about master, did not try stable)
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On 01/21/2017 07:34 AM, Enrico Forestieri wrote: > On Fri, Jan 20, 2017 at 06:49:29PM -0500, Richard Heck wrote: > >> On 01/20/2017 06:48 PM, Richard Heck wrote: >>> I'll cherry pick it for stable. >> No, I won't > Sorry, I would have to wait for an explicit nod... Sorry, that's not what I meant. I did say you should commit when you and JMarc had made a decision, so you had the nod. I had forgotten that it was a stable-only issue so thought you'd committed to master. Thanks for fixing this. I've been working on my logic notes for this semester, and there are a lot of \neq, which was very annoying. Richard
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On Fri, Jan 20, 2017 at 06:49:29PM -0500, Richard Heck wrote: > On 01/20/2017 06:48 PM, Richard Heck wrote: > > I'll cherry pick it for stable. > > No, I won't Sorry, I would have to wait for an explicit nod... -- Enrico
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On 01/20/2017 06:48 PM, Richard Heck wrote: > I'll cherry pick it for stable. No, I won't rh
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On 01/20/2017 06:37 PM, Enrico Forestieri wrote: > On Fri, Jan 20, 2017 at 07:53:40PM +0100, Enrico Forestieri wrote: >> On Thu, Jan 19, 2017 at 03:28:25PM -0500, Richard Heck wrote: >>> On 01/19/2017 02:35 PM, Enrico Forestieri wrote: 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. >>> Either of those would be fine with me. >> As I don't see what we gain from 2), I think 1) is the way to go. I attach >> here the amended patch. > Committed at 81465da5. I'll cherry pick it for stable. Richard >
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On Fri, Jan 20, 2017 at 07:53:40PM +0100, Enrico Forestieri wrote: > On Thu, Jan 19, 2017 at 03:28:25PM -0500, Richard Heck wrote: > > On 01/19/2017 02:35 PM, Enrico Forestieri wrote: > > > > > > 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. > > > > Either of those would be fine with me. > > As I don't see what we gain from 2), I think 1) is the way to go. I attach > here the amended patch. Committed at 81465da5. -- Enrico
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On Thu, Jan 19, 2017 at 03:28:25PM -0500, Richard Heck wrote: > On 01/19/2017 02:35 PM, Enrico Forestieri wrote: > > > > 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. > > Either of those would be fine with me. As I don't see what we gain from 2), I think 1) is the way to go. I attach here the amended patch. -- Enrico diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index f17ac37..83b2a1f 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -178,16 +178,27 @@ int GuiFontMetrics::width(docstring const & s) const int * pw = strwidth_cache_[s]; if (pw) return *pw; +#endif // For some reason QMetrics::width returns a wrong value with Qt5 - // int w = metrics_.width(toqstr(s)); + // with some arabic text. OTOH, QTextLayout is broken for single + // characters with null width (like \not in mathed). Also, as a + // safety measure, always use QMetrics::width with our math fonts. + int w = 0; + if (s.length() == 1 +#if QT_VERSION >= 0x040800 + || font_.styleName() == "LyX" #endif - QTextLayout tl; - tl.setText(toqstr(s)); - tl.setFont(font_); - tl.beginLayout(); - QTextLine line = tl.createLine(); - tl.endLayout(); - int w = int(line.naturalTextWidth()); + ) + 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
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On 01/19/2017 02:35 PM, Enrico Forestieri wrote: > 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. Either of those would be fine with me. Richard
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
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 cmsy127 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
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
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. 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? 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!) JMarc
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On Wed, Jan 18, 2017 at 03:19:31PM +0100, Jean-Marc Lasgouttes wrote: > > 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. > Is it Qt 4.8+ only? Yes, that method was introduced in Qt 4.8. > Will the bug exist on other systems with your > patch applied? Only if the fonts we use for math are not our owns. But if they are not, many other symbols would be broken. So, this would be only a minor issue. > But I see that you introduced this thing in f496ec3 and I presume that you > understand it! Actually, I introduced that style name in our fonts using fontforge. > Concerning master, applying your patch breaks \neq, presumably because > Guillaume worked around the problem manually. Possibly. But see below. > 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. I see that your patch is for master. I would advise to apply this patch only to stable (minus the lib/symbols bit, which is not needed there). This is because we know what is causing the issue in stable and thus the patch is fine. 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. As you can see, the \kern in the definition of \not is also in stable but it is not a problem there. My fear is that we put a patch on top of another patch without an understanding of what is happening, and this can bite later. -- Enrico
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
Le 17/01/2017 à 18:01, Enrico Forestieri a écrit : Hello Enroci, Hi JMcra :) :) I often do this typo, but usually I manage to correct it in time. Yes, but I would propose to do that in addition. Nonetheless, my feeling is that this issue is caused by the assumptions of Qt about a given font. For example, it refuses to draw the glyph of a character whose codepoint corresponds to a soft hyphen, irrespective of the actual font, and other such amenities. I am not completely sure what this styleName thing does. Is it mac/windows only? Is it Qt 4.8+ only? Will the bug exist on other systems with your patch applied? But I see that you introduced this thing in f496ec3 and I presume that you understand it! Concerning master, applying your patch breaks \neq, presumably because Guillaume worked around the problem manually. Here is what I came up with. Is it what you had in mind, or should the || in the test become a && ? You can see that the \kern in the definition of \not is gone, and this is definitely a good thing. JMarc diff --git a/lib/symbols b/lib/symbols index 82a6d13..2318d74 100644 --- a/lib/symbols +++ b/lib/symbols @@ -314,7 +314,7 @@ spadesuit cmsy127 170 mathord lyxnot cmsy 54 47 mathrel / hiddensymbol iffont cmsy # kerning is slightly imperfect so that one can see when \not is selected -\def\not{\lyxnot\mathrel{\kern-11mu}} +\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 a0fa699..2f598f4 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -182,17 +182,28 @@ int GuiFontMetrics::width(docstring const & s) const int * pw = strwidth_cache_[s]; if (pw) return *pw; - // For some reason QMetrics::width returns a wrong value with Qt5 - // int w = metrics_.width(toqstr(s)); PROFILE_CACHE_MISS(width) #endif - QTextLayout tl; - tl.setText(toqstr(s)); - tl.setFont(font_); - tl.beginLayout(); - QTextLine line = tl.createLine(); - tl.endLayout(); - int w = int(line.naturalTextWidth()); + /* For some reason QMetrics::width returns a wrong value with Qt5 + * with some arabic text. OTOH, QTextLayout is broken for single + * characters with null width (like \not in mathed). + */ + int w = 0; + 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
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On Tue, Jan 17, 2017 at 02:51:23PM +0100, Jean-Marc Lasgouttes wrote: > Le 17/01/2017 à 12:19, Enrico Forestieri a écrit : > > This seems to be a Qt issue. QTextLine::naturalTextWidth() should report > > the width of the line occupied by text. If a zero-width character is > > present, it is correctly accounted for, except when it is the only > > character in the line. That is to say that "/=" has the same width > > as "=" if "/" has zero-width, but "/" by alone is reported to have > > its real width. The metrics in math are computed char by char, so that > > explains the behavior. The solution is to compute the width by using > > the old method when when the font is one of ours. See attached patch. > > Hello Enroci, Hi JMcra :) > Thanks for the analysis. You patch is a solution, but wouldn't it more in > line with what you propose to use width(s[0]) when there is only one > character? Yes, but I would propose to do that in addition. Nonetheless, my feeling is that this issue is caused by the assumptions of Qt about a given font. For example, it refuses to draw the glyph of a character whose codepoint corresponds to a soft hyphen, irrespective of the actual font, and other such amenities. > JMarc > -- Enrico
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
Le 17/01/2017 à 15:38, Richard Heck a écrit : Thanks for your work on this, both of you. When you decide on a solution, please feel free to commit to stable. OK, thanks. While we are at it, is there some reason why I cannot get an answer there ? http://www.mail-archive.com/lyx-devel@lists.lyx.org/msg198350.html JMarc
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On 01/17/2017 08:51 AM, Jean-Marc Lasgouttes wrote: > Le 17/01/2017 à 12:19, Enrico Forestieri a écrit : >> This seems to be a Qt issue. QTextLine::naturalTextWidth() should report >> the width of the line occupied by text. If a zero-width character is >> present, it is correctly accounted for, except when it is the only >> character in the line. That is to say that "/=" has the same width >> as "=" if "/" has zero-width, but "/" by alone is reported to have >> its real width. The metrics in math are computed char by char, so that >> explains the behavior. The solution is to compute the width by using >> the old method when when the font is one of ours. See attached patch. > > Hello Enroci, > > Thanks for the analysis. You patch is a solution, but wouldn't it more > in line with what you propose to use width(s[0]) when there is only > one character? Thanks for your work on this, both of you. When you decide on a solution, please feel free to commit to stable. Richard
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
Le 17/01/2017 à 12:19, Enrico Forestieri a écrit : This seems to be a Qt issue. QTextLine::naturalTextWidth() should report the width of the line occupied by text. If a zero-width character is present, it is correctly accounted for, except when it is the only character in the line. That is to say that "/=" has the same width as "=" if "/" has zero-width, but "/" by alone is reported to have its real width. The metrics in math are computed char by char, so that explains the behavior. The solution is to compute the width by using the old method when when the font is one of ours. See attached patch. Hello Enroci, Thanks for the analysis. You patch is a solution, but wouldn't it more in line with what you propose to use width(s[0]) when there is only one character? JMarc
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
On Mon, Jan 16, 2017 at 03:36:07PM -0500, Richard Heck wrote: > Resending as a new thread so it is more visible > > On 01/16/2017 02:04 PM, Scott Kostyshak wrote: > > On Mon, Jan 16, 2017 at 01:13:44PM -0500, Richard Heck wrote: > >> I am seeing display problems in stable with \notin and \noteq. The > >> latter, for example, shows (In LyX) as "/=", rather than as ≠. Similarly > >> for \notin. See the attachments. > > Just a few data points: > > > > I can reproduce the problem on current 2.2.x branch. > > I cannot reproduce on master or on 2.2.1. > > Bisect blames: > > 24648404b3c85015584b1ca127e257cbecf3342d is the first bad commit > commit 24648404b3c85015584b1ca127e257cbecf3342d > Author: Jean-Marc Lasgouttes> Date: Sun Oct 23 20:52:01 2016 +0200 This seems to be a Qt issue. QTextLine::naturalTextWidth() should report the width of the line occupied by text. If a zero-width character is present, it is correctly accounted for, except when it is the only character in the line. That is to say that "/=" has the same width as "=" if "/" has zero-width, but "/" by alone is reported to have its real width. The metrics in math are computed char by char, so that explains the behavior. The solution is to compute the width by using the old method when when the font is one of ours. See attached patch. > Strange, though, that this does not cause the same problem in master. > Probably > that is due to other changes JMarc has made there. I did not investigate why that is not a problem in master. -- Enrico diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index f17ac37..b6761c9 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -181,13 +181,18 @@ 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 (font_.styleName() == "LyX") { + 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
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
Le 16/01/2017 à 22:09, Jean-Marc Lasgouttes a écrit : Le 16/01/2017 à 21:29, Richard Heck a écrit : Bisect blames: 24648404b3c85015584b1ca127e257cbecf3342d is the first bad commit commit 24648404b3c85015584b1ca127e257cbecf3342d Author: Jean-Marc LasgouttesDate: Sun Oct 23 20:52:01 2016 +0200 Work around issues with Qt5 and Arabic text Thanks Richard, I'll have a look. Guillaume, any intuition why master is different from stable in this respect? I can confirm that using QFontMetrics::width instead of the width returned by QTextLayout does the trick, but that makes me a bit nervous :) What happens basically is that \not has 0 width, but QTextLayout does not seem to respect that. Several workarounds exist - have a different FontMetrics::width for maths - in mathed_string_dim, use width(char_type) repeatedly instead of width(docstring) - or do a shortcut for cases where the string only has one character. All these things would work, but they are fragile as long as we do not know: - why they work - why is master immune to the problem Guillaume, could it just be that the definition of \not needs to be adjusted like it has been in master? JMarc
Re: [CONFIRMED] Display Problem in Stable with \notin, etc.
Le 16/01/2017 à 21:29, Richard Heck a écrit : Bisect blames: 24648404b3c85015584b1ca127e257cbecf3342d is the first bad commit commit 24648404b3c85015584b1ca127e257cbecf3342d Author: Jean-Marc LasgouttesDate: Sun Oct 23 20:52:01 2016 +0200 Work around issues with Qt5 and Arabic text Thanks Richard, I'll have a look. Guillaume, any intuition why master is different from stable in this respect? JMarc
[CONFIRMED] Display Problem in Stable with \notin, etc.
Resending as a new thread so it is more visible On 01/16/2017 02:04 PM, Scott Kostyshak wrote: > On Mon, Jan 16, 2017 at 01:13:44PM -0500, Richard Heck wrote: >> I am seeing display problems in stable with \notin and \noteq. The >> latter, for example, shows (In LyX) as "/=", rather than as ≠. Similarly >> for \notin. See the attachments. > Just a few data points: > > I can reproduce the problem on current 2.2.x branch. > I cannot reproduce on master or on 2.2.1. Bisect blames: 24648404b3c85015584b1ca127e257cbecf3342d is the first bad commit commit 24648404b3c85015584b1ca127e257cbecf3342d Author: Jean-Marc LasgouttesDate: Sun Oct 23 20:52:01 2016 +0200 Work around issues with Qt5 and Arabic text This fixes two particular problems * with Qt5, it seems that QFontMetrics::width does not return the correct value for some Arabic text; this patch uses QTextLayout instead to compute a string width * Likewise, the undocumented layout flags TextForceRightToLeft and TextForceLeftToRight do not work with Arabic text; this patch uses unicode override characters instead. It might be that the two issues are related. In any case, they do not happen with latin text where right-to-left direction is enforced. And they do not happen with Qt4. Additionally, remove some dead code in GuiFontMetrics::pos2x(). Fixes bug #10436. (cherry picked from commit e832d2e90f300afb1b1255a486e56d059b2dfab7) Strange, though, that this does not cause the same problem in master. Probably that is due to other changes JMarc has made there. Richard
[CONFIRMED] Display Problem in Stable with \notin, etc.
On 01/16/2017 02:04 PM, Scott Kostyshak wrote: > On Mon, Jan 16, 2017 at 01:13:44PM -0500, Richard Heck wrote: >> I am seeing display problems in stable with \notin and \noteq. The >> latter, for example, shows (In LyX) as "/=", rather than as ≠. Similarly >> for \notin. See the attachments. > Just a few data points: > > I can reproduce the problem on current 2.2.x branch. > I cannot reproduce on master or on 2.2.1. Bisect blames: 24648404b3c85015584b1ca127e257cbecf3342d is the first bad commit commit 24648404b3c85015584b1ca127e257cbecf3342d Author: Jean-Marc LasgouttesDate: Sun Oct 23 20:52:01 2016 +0200 Work around issues with Qt5 and Arabic text This fixes two particular problems * with Qt5, it seems that QFontMetrics::width does not return the correct value for some Arabic text; this patch uses QTextLayout instead to compute a string width * Likewise, the undocumented layout flags TextForceRightToLeft and TextForceLeftToRight do not work with Arabic text; this patch uses unicode override characters instead. It might be that the two issues are related. In any case, they do not happen with latin text where right-to-left direction is enforced. And they do not happen with Qt4. Additionally, remove some dead code in GuiFontMetrics::pos2x(). Fixes bug #10436. (cherry picked from commit e832d2e90f300afb1b1255a486e56d059b2dfab7) Strange, though, that this does not cause the same problem in master. Probably that is due to other changes JMarc has made there. Richard