Bug Hunting Session
Bug 48256 - clean out Hide/Show Cursor ...
Summary: clean out Hide/Show Cursor ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Master old -3.6
Hardware: Other All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:3.7.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2012-04-03 09:38 UTC by Michael Meeks
Modified: 2015-12-15 23:50 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Proposed patch (7.49 KB, patch)
2012-06-08 21:01 UTC, Thomas Arnhold
Details
Proposed patch Part 2 (1.81 KB, patch)
2012-06-08 21:03 UTC, Thomas Arnhold
Details
Proposed patch Part 3 (3.58 KB, patch)
2012-06-08 21:13 UTC, Thomas Arnhold
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2012-04-03 09:38:02 UTC
In the bad old days of calc, everything was rendered explicitly to the screen, and then the cursor was rendered over the top using XOR rendering. To get this right, before we rendered new things we would need to call:

HideCursor();
// do the incremental render
ShowCursor();

These days, we should never do that - all rendering should be done in response to an idle handler, and should paint from scratch instead of trying to be 'clever' like this (it will increasingly not work & just waste resources on  modern systems).

So - we need to audit the code for these Hide/Show pairs. Clearly some instances of hiding the cursor are prolly still applicable, but we should check all of them in sc/ 'git grep -5 HideCursor' - and remove and re-test all instances.

Potentially we want to add some calls to Invalidate() on the cursor region when this causes problems :-)

Thanks !
Comment 1 Florian Reisinger 2012-05-18 09:29:37 UTC
Deleted "Easyhack" from summary.
Comment 2 Thomas Arnhold 2012-06-08 21:01:25 UTC
Created attachment 62830 [details]
Proposed patch
Comment 3 Thomas Arnhold 2012-06-08 21:03:52 UTC
Created attachment 62831 [details]
Proposed patch Part 2

For my understanding CursorSwitcher does the same as Show/HideCursor only in the way that aCursorSwitch gets destructed after the method end (and so hidden).
Comment 4 Thomas Arnhold 2012-06-08 21:13:03 UTC
Created attachment 62832 [details]
Proposed patch Part 3
Comment 5 Michael Meeks 2012-06-26 04:30:50 UTC
Looks lovely to me; if you did some testing & things seems to work fine - please do push them to master :-) it is possible that at some sites we might want to queue an idle re-draw of that area but presumably we can add that later if there are issues.

Thanks !
Comment 6 Caolán McNamara 2012-06-28 08:15:32 UTC
What's the situation with this patch, should it go in, or do you want someone to extra review it, or did something equivalent go in already ?
Comment 7 Thomas Arnhold 2012-06-29 20:48:22 UTC
Caolán: Yes it would be nice if someone else could review it.
Comment 8 Caolán McNamara 2012-06-29 23:14:47 UTC
caolanm->erack/kohei: one of you guys take this under your wing ?
Comment 9 Kohei Yoshida 2012-07-02 10:48:15 UTC
(In reply to comment #8)
> caolanm->erack/kohei: one of you guys take this under your wing ?

IMO Michael's review in Comment 5 should be more than sufficient (since this is his EasyHack).  If Thomas needs extra assurance I can give mine.  Thomas, please push your changes to master.  Thanks!
Comment 10 Not Assigned 2012-07-13 11:15:06 UTC
Thomas Arnhold committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: fdo#48256 clean out Hide/Show Cursor
Comment 11 Caolán McNamara 2012-07-13 11:18:01 UTC
alright, so I pushed it. I'll leave it to the calc guys to fix things up if there's any problems with it :-)
Comment 12 Robinson Tryon (qubit) 2015-12-15 23:50:46 UTC
Migrating Whiteboard tags to Keywords: (EasyHack,DifficultyBeginner,SkillCpp,TopicCleanup)
[NinjaEdit]