Bug 96971 - SetXORMode - mac requires apparently un-needed parameter ...
Summary: SetXORMode - mac requires apparently un-needed parameter ...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
5.1.0.1 rc
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.2.0.1
Keywords:
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2016-01-08 21:36 UTC by Michael Meeks
Modified: 2023-05-10 17:47 UTC (History)
4 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 Michael Meeks 2016-01-08 21:36:10 UTC
If you do a:

git grep -5 SetXORMode

in VCL - you'll see that the second parameter:

virtual void SetXORMode( bool bSet, bool bInvertOnly )

bInvertOnly - appears to only be implemented for OSX - and all the other backends just use 'bSet' - to mean "XOR" vs. "Not XOR".

As such - it'd be great to go back through the callees and check that no-one is really using bInvertOnly in anger, and then strip that parameter out of the backend APIs, and also out of the VCL API in each instance; and ideally out of any callers that use bInvertOnly.

That is - unless there is indeed a use-case for this =) (I suspect not).

Thanks !
Comment 1 Michael Meeks 2016-01-08 21:36:55 UTC
Armin - quick sanity check appreciated for this easy hack =)
Comment 2 Armin Le Grand 2016-01-11 15:03:15 UTC
Sounds good - but you already said it's used on the Mac - so how to remove it then?
Comment 3 Michael Meeks 2016-01-11 15:23:32 UTC
Oh - well; its present in all the method calls, but implemented but only for the Mac; which makes me wonder - is it truly used and/or useful ? =)
Comment 4 Armin Le Grand 2016-01-12 12:18:00 UTC
Sorry, Michael, cannot say if it's really used on the Mac ;-( You'll have to deep-dive...
Comment 5 Armin Le Grand 2016-01-12 12:22:00 UTC
Maybe one more note: Of course XOR is bad and should be removes ASAP - that is why I already use overlay for selections in apps (Writer/draw/impress/calc) which use the system-selected selection color and a blending. Bad stuff remaining is a running EditEngine (also outline mode in Impress) and single/multiLine EditFields...
For EditEngine my dream is to have/migrate to one that just layouts to primitives (already done for paints), separate editor part that visualizes the selection as overlay. Some work to do...
Comment 6 kerem 2016-02-02 07:01:55 UTC
I sent following for this patch;

https://gerrit.libreoffice.org/#/c/22003/
Comment 7 Robinson Tryon (qubit) 2016-02-18 14:52:11 UTC Comment hidden (obsolete)
Comment 8 Commit Notification 2016-06-09 11:32:36 UTC
melikeyurtoglu committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=258301879bcd20397c38bbd522dea2c923bd9fc2

tdf#96971 SetXORMode - remove un-needed parameter

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 jani 2016-06-14 09:49:44 UTC
Look solved
Comment 10 Commit Notification 2018-12-05 07:19:26 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/be504e8c7ef06637f055c43ad350381377df1e90%5E%21

tdf#121719: Revert fix for tdf#96971

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Xisco Faulí 2018-12-05 11:51:08 UTC
@Michael Meeks, should we reopen this easyHack?
Is there any way we could clean the code without breaking the mac code ?
Comment 12 Michael Meeks 2018-12-05 11:56:11 UTC
No idea - we should prolly put this on Tor's list of things to look at to stop Mac holding us back here in the longer term.

For now - lets stop it being an easy-hack; and re-title it =)

Thanks Xisco.
Comment 13 Commit Notification 2018-12-05 18:00:05 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "libreoffice-6-2":

https://git.libreoffice.org/core/+/708da2a6342a58fe379217064848c1c311216f05%5E%21

tdf#121719: Revert fix for tdf#96971

It will be available in 6.2.0.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 QA Administrators 2019-12-07 03:42:57 UTC Comment hidden (obsolete)
Comment 15 QA Administrators 2021-12-07 04:57:19 UTC
Dear Michael Meeks,

To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year.

There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present.

If you have time, please do the following:

Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/

If the bug is present, please leave a comment that includes the information from Help - About LibreOffice.
 
If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice.

Please DO NOT

Update the version field
Reply via email (please reply directly on the bug tracker)
Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not 
appropriate in this case)


If you want to do more to help you can test to see if your issue is a REGRESSION. To do so:
1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/

2. Test your bug
3. Leave a comment with your results.
4a. If the bug was present with 3.3 - set version to 'inherited from OOo';
4b. If the bug was not present in 3.3 - add 'regression' to keyword


Feel free to come ask questions or to say hello in our QA chat: https://web.libera.chat/?settings=#libreoffice-qa

Thank you for helping us make LibreOffice even better for everyone!

Warm Regards,
QA Team

MassPing-UntouchedBug