Bug 96826 - Typewriter attribute given too much weight when finding font based on attributed
Summary: Typewriter attribute given too much weight when finding font based on attributed
Status: RESOLVED INVALID
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium minor
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-30 08:53 UTC by Chris Sherlock
Modified: 2021-10-06 08:24 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Sherlock 2015-12-30 08:53:13 UTC
PhysicalFontFamily::FindFontFamilyByAttributes() appears to give a weight for typewriter attributes that is too low. 

According to the comment in the source, the weighting should be:

// Calculate Match Value
// 1000000000
//  100000000
//   10000000   CJK, CTL, None-Latin, Symbol
//    1000000   FamilyName, Script, Fixed, -Special, -Decorative,
//              Titling, Capitals, Outline, Shadow
//     100000   Match FamilyName, Serif, SansSerif, Italic,
//              Width, Weight
//      10000   Scalable, Standard, Default,
//              full, Normal, Knownfont,
//              Otherstyle, +Special, +Decorative,
//       1000   Typewriter, Rounded, Gothic, Schollbook
//        100

The code starting at line 606 of PhysicalFontCollection.cxx, however, shows that the calculation is off by a factor of 200:

    609         // test MONOSPACE+TYPEWRITER attributes
    610         if( nSearchType & ImplFontAttrs::Fixed )
    611         {
    612             if( nMatchType & ImplFontAttrs::Fixed )
    613                 nTestMatch += 1000000*2;
    614             // a typewriter attribute is even better
    615             if( ImplFontAttrs::None == ((nSearchType ^ nMatchType) & ImplFontAttrs::Typewriter) )
    616                 nTestMatch += 10000*2;
    617         }

However, I believe that if the font has the type ImplFontAttrs::Typewriter, then it is just like ImplFontAttrs::Fixed and should have an additional weight of at least 1_000_000, and due to the extra weight this is being given it should be 2_000_000.

I've literally dug into the source history like an archeologist, and this looks like a regressions from a long, long while ago... This code was originally in vcl/source/gdi/outdev3.cxx, and on 2001-06-29 (!!) in commit 925806de64f97d3b11a552143d3dbd42648f64cc it looks strongly like the weighting was typed in wrongly...

Link to commit in cgit:

http://cgit.freedesktop.org/libreoffice/core/commit/vcl/source/gdi/outdev3.cxx?id=925806de64f97d3b11a552143d3dbd42648f64cc

Anyone able to confirm if this behaviour is off? This is *very* obscure if it is a bug, hence I've marked this as minor.
Comment 1 Chris Sherlock 2016-02-23 13:42:05 UTC
Argh! Misreading that code! *Typewriter* is meant to have a weight 1_000, not 10_000 like it shows in that snippet. All that effort reading the code, and I went home and misremembered the issue I saw.

I'm going to fix this when I get home from hospital. It's a definite mistake.
Comment 2 Xisco Faulí 2016-09-20 16:10:38 UTC
Adding keyword 'bibisectRequest' to see whether this regression is already present in the oldest build of bibisect-43all repository or not.
In case it's already present, change 'bibisectRequest' to 'preBibisect'.
Otherwise, change 'bibisectRequest' to 'bibisected' and add a comment with the output from 'git bisect log'
Comment 3 Xisco Faulí 2017-09-29 08:53:23 UTC Comment hidden (obsolete)
Comment 4 Buovjaga 2018-05-25 11:22:33 UTC
Version is 3.3 so bibisectRequest does not make sense.
Comment 5 Xisco Faulí 2019-05-15 10:17:21 UTC
it can't be a regression if it's inherit from OOo
Comment 6 QA Administrators 2021-05-15 04:18:20 UTC Comment hidden (obsolete)
Comment 7 Chris Sherlock 2021-10-05 16:22:18 UTC
Pleas close as WONTFIX. I have since delved into the code (yes, I realise this has taken 6 years) and found this is not an issue. However, I did find some other bugs tangential to this one that I am working on.
Comment 8 Chris Sherlock 2021-10-05 16:22:36 UTC
Sorry, please make it closed as INVALID.