Bug 74424 - Start to decouple Window from OutputDevice
Summary: Start to decouple Window from OutputDevice
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:4.3.0 target:7.1.0
Keywords: difficultyMedium, easyHack, skillCpp, topicCleanup
Depends on: 74702
Blocks:
  Show dependency treegraph
 
Reported: 2014-02-03 04:19 UTC by Chris Sherlock
Modified: 2021-04-05 15:05 UTC (History)
2 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 2014-02-03 04:19:52 UTC
We want to start to decouple Window from OutputDevice. This is because a Window should *use* an OutputDevice, a Window is *not* an OutputDevice. With such a tight coupling of really unrelated classes it makes it hard to write unit tests, extend the code and it violates the single responsibility principle.

In order to start to decouple the two classes, we need to change from inheritance to composition. I propose that the strategy should be: 

1. Add in a new private member pOutputDevice as well as an accessor. When the Window is initialized, pOutputDevice is set to a downcasted instance of this (because we are still inheriting Window from OutputDevice). 

2. Start to go through all the functions in Window that rely on OutputDevice and use the getter function to use the OutputDevice private member. 

3. Any subclasses of Window will then need to have the same procedure done on them. 

4. Once this is done, we then need to work out the best way of initializing mpOutputDevice, then we can remove OutputDevice as the parent class of Window (and all Window subclasses).
Comment 1 Commit Notification 2014-02-03 16:45:38 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0de3b50ff85f8a2ca1fed00c20a68cb51622dc71

fdo#74424 OutputDevice no longer inherits Resource



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 2 Commit Notification 2014-02-05 13:42:17 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6a6a51ed0546de2d6e198b0d7486d347b2fb345b

fdo#74424 Start to decouple Window class from OutputDevice



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 3 Commit Notification 2014-02-05 14:17:27 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplGetDPI(X|Y)()



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 4 Commit Notification 2014-02-05 14:24:59 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplReMirrored()



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 5 Commit Notification 2014-02-05 14:27:19 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplIsRecordLayout()



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 6 Commit Notification 2014-02-05 14:29:46 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplReleaseGraphics()



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 7 Commit Notification 2014-02-05 14:31:17 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplSelectClipRegion()



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 8 Commit Notification 2014-02-05 14:32:36 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7e122c54f588f56d45e7c09a3327fecb0f8dbef8

fdo#74424 Use Window::GetOutDev() to access ImplInitFontList()



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 Commit Notification 2014-02-05 14:39:13 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplLogicToDevicePixel()



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 10 Commit Notification 2014-02-05 14:42:37 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 Use Window::GetOutDev() to access ImplGetGraphics()



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 11 Commit Notification 2014-02-05 14:43:54 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8f542a605baec0b777effbb3e6666eff09a09a28

fdo#74424 Use Window::GetOutDev() to access ImplDrawFrameDev()



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 12 Commit Notification 2014-02-05 14:52:33 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9cb1018c81dbceead9ee2f4da26989035c5a0bab

fdo#74424 Use Window::GetOutDev() to access ImplInitOutDevData()



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 13 Commit Notification 2014-02-05 14:53:51 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

fdo#74424 - added TODO



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 14 Commit Notification 2014-02-05 15:00:20 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=999a83b455b9db32116203fcc7337f9910d1d5f7

fdo#74424 Use Window::GetOutDev() to access ImplHasMirroredGraphics()



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 15 Commit Notification 2014-02-10 20:39:25 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=19c5867f922e9acf0564b6213fa15cc899a77c16

fdo#74424 HasMirroredGraphics changes



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 16 Xisco Faulí 2016-09-10 15:50:24 UTC
Hi Chris,
I'm setting this ticket back to NEW as it has been inactive for more than 3
months.
Feel free to assign it back to you if you're still working on this.
Regards
Comment 17 Commit Notification 2020-06-29 20:00:17 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/49d6768483fbae8789d97512e78a00b860fbad60

tdf#74424 vcl: Flush() is implemented in OutputDevice so no need for type check

It will be available in 7.1.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 18 Chris Sherlock 2021-01-30 06:53:56 UTC
There are better approaches to this. I am closing this bug as I think it is probably confusing newbies.
Comment 19 Buovjaga 2021-04-05 15:05:17 UTC
(In reply to Chris Sherlock from comment #18)
> There are better approaches to this. I am closing this bug as I think it is
> probably confusing newbies.

Looks like there is now a megapatch for the rest: https://gerrit.libreoffice.org/c/core/+/113204