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:
 
Reported: 2016-01-08 21:36 UTC by Michael Meeks
Modified: 2018-12-06 13:11 UTC (History)
5 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.