Bug 161837 - Drop Fraction class from tools
Summary: Drop Fraction class from tools
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyMedium, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2024-06-29 09:23 UTC by Mike Kaganski
Modified: 2025-01-08 05:08 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 Mike Kaganski 2024-06-29 09:23:34 UTC
In the deprecated tools library, there is a Fraction class (<tools/fract.hxx>), that is used rather extensively, e.g. in VCL.

It has problems, including performance: e.g. it is built using boost/rational, but to avoid dependency of each library using the class on boost, it's used in the tools' CXX, so no inlining. But the most problematic is its unreliable nature. It has a 'mbValid' member, which may be set to false on an intermediate overflow; and then, the invalid fraction breaks all calculations.

Consider a valid fraction with numerator 72863296 and denominator 92331675, approximately equal to 0.7891. Trying to multiply number 193 by this fraction will give 0:

    Fraction f(72'863'296, 92'331'675);
    assert(f.IsValid());
    auto x = (Fraction(193) * f).operator sal_Int32();

Here, x is suppose to be 152, but will be 0, because multiplying 193 by 72'863'296 gives 14'062'616'128, which overflows the underlying sal_Int32, setting the multiplication result to invalid (see Fraction::operator *= in tools/source/generic/fract.cxx).

Of course, it is possible to fix such things one by one, by introducing more checks, branching the code to handle different cases, and hoping that now it will give no surprises, until a next case is found. But these days, floating point calculations are not much slower than integer; and given the lack of inlining, rather complex logic and fragility, the use of Fraction seems to slow down, introduce errors, and give no upsides at all.

This task is to replace all uses of Fraction in the codebase by doubles; and in the end, drop the Fraction code from tools. This is a task that implies many developers, so should not be assigned to yourself.
Comment 1 Hossein 2024-07-10 14:04:34 UTC
Assigning the issue to myself as per discussion on IRC. I am trying to do this change in one round, or at least do the VCL part in that. This is what I have done, which is not complete yet:

tdf#161837 Use double instead of Fraction class
https://gerrit.libreoffice.org/c/core/+/170084
Comment 2 Buovjaga 2024-11-07 05:17:41 UTC
Per the discussion in the patch, looks like this should not be assigned anymore, but done in smaller bits.
Comment 3 Samuel Adesola 2025-01-08 00:59:22 UTC
If this will be taken bit by bit, how easy can one solve for code dependency?, I started working on the sw/source/core/view folder. what I noticed is that if some changes are made to double, especially in the case of a function argument or class constructor, one has to go back to where such a function is defined and change the parameter type, but the issue is there might be some file somewhere using the same function and is still using Fraction as the argument. 

Is there a simple conversion between double and Fraction that one can use? While working on the view folder, one method I thought of is to overload the functions that I might need to change so I can still have the one that uses Fraction so as to get the code to compile. If this is a good approach, I plan to work on a folder until I have traced back everywhere where the function I overloaded is used and eventually remove the Fraction overload definition.

A WIP where I did this can be found here: https://gerrit.libreoffice.org/c/core/+/179921
Comment 4 Mike Kaganski 2025-01-08 05:08:39 UTC
(In reply to Samuel Adesola from comment #3)
> Is there a simple conversion between double and Fraction that one can use?

If I were working on this task, I think I would try first to look at the definition of the Fraction class; likely I would find include/tools/fract.hxx, where in the definition of the class, I could see 'explicit operator double() const', hinting me, that everywhere in a code like

  x = foo(aFract);

I could change it to

  x = updated_foo(double(aFract));

HTH