View Issue Details

IDProjectCategoryView StatusLast Update
0014257LazarusLCLpublic2019-04-22 19:33
ReporterMartin Friebe Assigned ToG. Colla  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version0.9.27 (SVN) 
Target Version1.4 
Summary0014257: SetCursor faulty under GTK2 => changes cursor for wrong control
Description- Create a project with an empty form.
- put a SpeedButton (or other TGraphicControl) on it.
- Add a handler
procedure TForm1.FormClick(Sender: TObject);
begin
  SpeedButton1.Cursor:=crIBeam;
end;

Run, and click on the form (do not move the mouse)

The cursor will turn into IBeam. But it should not, since you are not over the Form, not over the SpeedButton.


The Problem is caused, because TGraphicControl do not have there own cursor with the widgetset.
So TControl.SetCursor calls TControl.SetTempCursor, whcih calls Parent.SetCursor (equals TWinControl.SetCursor)

TWinControl then instructs the widgetset to set the cursor for self (for the WinControl)

This should only be done, if the mouse-pointer is above the Control for which the cursor has changed.

The call to SetTempCursor must be kept. It ensures the Cursor is changed even if the mouse is not moved. This is important if an application wants for example to hide the Cursor (like many Textprocessors hide it when text is entered, and show it on mouse-move => so hiding can and must not rely on Mouse-Move)

SetTempCursor should include the calling control.

The check could either be done in the LCL, easy for all widgetsets. (Windows will have to implement immediate setting as well, for the above reason), or the Control could be passed to the WidgetSet, and the check could be done there

Alternatively, the widgetset SetCursor (which is given the WinControl and the TCursor value)could ignore the Value for the cursor, and ask the wincontrol for the control at the mouse-position
TagsNo tags attached.
Fixed in Revision47841
LazTarget1.4
WidgetsetGTK 2
Attached Files

Activities

Zeljan Rikalo

2010-09-19 21:02

developer   ~0041210

Lazarus 0.9.29 r27414 FPC 2.4.1 i386-linux-qt (beta) both Qt and Gtk2 shows same behaviour. I'm wondering is this ws or lcl bug ? if WS then what could be the reason ?

Zeljan Rikalo

2011-04-04 08:09

developer   ~0047158

@Martin, what we'll do with this one ?

Martin Friebe

2011-04-05 13:47

manager   ~0047245

I think it's actually LCL (though I have not really checked all aspects)

The win widgetset just probably is smart to do extra checks.

In WS only a WinControl has a cursor.

So other Controls do SetTempCursor on their parent...; that is correct, IF hte mouse is above them.

If a control receives SetCursor, it does;
  Perform(CM_CURSORCHANGED, 0, 0)

Which leads to setting temp cursor.
  TControl.SetTempCursor
  TWinControl.SetTempCursor
And that calls SetCursor on the widgetset (passing in the wincontrol, and the cursor desired)

The windows widgetset, ignores the passed in param, and finds the control (control, not only wincontrol) from real mouse coordinates)

So that's why it works on windows....

As for fixing it. that will require a lot of testing.
It is likely that a lot of code reacts sensitive to this.

- Cursor for controls in scrollboxes (there is an additional bug there)
- the ability to change the cursor without mouse move (SynEdit uses that, if it hides the cursor when the user starts typing)




the windows widgetset deals with that wrong call

Martin Friebe

2011-04-05 13:49

manager   ~0047246

Note that cursor handling also has a lot of overhead... (at least did have)

on windows you have WM_CURSOR, whic is send with every mouse move.

But since other widgetsets do not have that, cursor is updated in the mousemove event.

Not sure if windows does both...

Zeljan Rikalo

2012-02-03 11:42

developer   ~0056285

@Martin can you review this issue ? cursor behaviour is changed few 1-2 months ago and it should work now.

Zeljan Rikalo

2012-02-04 13:14

developer   ~0056377

Not blocker, postponed.

Martin Friebe

2012-02-05 01:40

manager   ~0056502

Problem still exists.

Zeljan Rikalo

2012-02-05 12:39

developer   ~0056509

So according to your previous notes it needs a lot of changes and testing inside lcl, postponing is good alternative atm.

Bart Broersma

2015-01-31 16:50

developer   ~0080700

Is an approach like this feasible at all?
Only call parent.SetTempCursor if mouse is above the control we want to change the cursor for?

procedure TControl.SetTempCursor(Value: TCursor);
var
  Pt: TPoint;
  Ct: TControl;
begin
  if Parent<>nil then
  begin
    Pt := Parent.ScreenToClient(Mouse.CursorPos);
    Ct := Parent.ControlAtPos(Pt, True);
    if (Self = Ct) then Parent.SetTempCursor(Value);
  end;
end;

A little test in GTK works, but I can easily oversee side-effects.

G. Colla

2015-02-13 11:39

developer   ~0081023

There are 4 possible solutions to the problem:

1) your patch. It affects, and might break some compatibility of all TControl descendants, i.e of all controls.

2) my patch, i.e. to override TGraphicContro.SetCursor, moving there your test before calling SetTempCursor. It affects, and might break compatibility of some 3d party components descendants of TGraphicControl (i.e. a much smaller subset of controls)

3) patching at widgetset level, by adding to all widgetset the check which currently is performed only in the WIN WS.Requires a revision and test of all widgetsets.

4) Martin's suggestion to add a listener to CM_CURSORCHANGED in behalf of TGraphicControl and perform the test there (to be tested).

Bart Broersma

2015-02-13 14:24

developer   ~0081026

> 1) your patch. It affects, and might break some compatibility of all TControl
> descendants, i.e of all controls

Well only if the descendants override SetTempCursor and do not call inherited.
I fail to see how the logic in my proposal could break other controls.
We only need to set th etemporary cursor if th emouse is over the control in the first place.
It might not be the most logical or effective place to do so, but that's another question entirely IMO.
The extra check may cost some time, but any code that calls setcursor so often that this will slow down the program should be redisigned in the first place.

G. Colla

2015-02-13 23:52

developer   ~0081048

It may break compatibility of any control just using SetTempCursor, and not expecting it to be blocked by an additional check, which is not guaranteed to be appropriate in any conceivable condition.
What is a bug when SetTempCursor is called as a consequence of calling SetCursor may be a feature if SetTempCursor is called directly in other cases.

Moving your check to a safer area (a listener of CM_CURSORCHANGED specific for TGraphicControl) restricts the scope of the change just to the specific case, and therefore minimizes the risks.

G. Colla

2015-02-16 00:50

developer   ~0081121

Added a listener for CM_CURSORCHANGED in TGraphicControl performing the test
provided by Bart's patch.

resolved in rev. 47814

G. Colla

2015-02-17 00:44

developer   ~0081155

Further tests have shown a flaw when the TGraphicControl is on a TScrollBox.

ControlAtPos uses physical Control coordinates (actual position inside parent), while ScreenToClient returns logical control coordinates (coordinates when offset=0) Therefore the check fails when parent is a scrolling control with offset <> 0;
ScreenToClient must be replaced with ScreenToControl for consistency with ControlAtPos.

G. Colla

2015-02-17 00:45

developer   ~0081156

Change committed in rev. 47841

Zeljan Rikalo

2015-02-17 07:53

developer   ~0081159

@Giuliano, when you commit an resolve issue pls write revision in "fixed in revision" field.

Zeljan Rikalo

2015-02-17 08:33

developer   ~0081160

Added r47841 into "Fixed in Revision" field.

G. Colla

2015-02-17 09:46

developer   ~0081161

@Zeljko - Sorry, will do in future. Thnks.

Issue History

Date Modified Username Field Change
2009-08-03 01:41 Martin Friebe New Issue
2009-08-03 01:41 Martin Friebe LazTarget => -
2009-08-03 01:41 Martin Friebe Widgetset => GTK 2
2009-08-03 01:44 Martin Friebe Description Updated
2009-08-03 13:44 Vincent Snijders LazTarget - => 1.0
2009-08-03 13:44 Vincent Snijders Status new => acknowledged
2009-08-03 13:44 Vincent Snijders Target Version => 1.0.0
2010-09-19 21:02 Zeljan Rikalo Note Added: 0041210
2011-04-04 08:09 Zeljan Rikalo Note Added: 0047158
2011-04-04 08:09 Zeljan Rikalo Status acknowledged => feedback
2011-04-05 13:47 Martin Friebe Note Added: 0047245
2011-04-05 13:49 Martin Friebe Note Added: 0047246
2012-02-03 11:42 Zeljan Rikalo Note Added: 0056285
2012-02-03 11:42 Zeljan Rikalo Status feedback => acknowledged
2012-02-04 13:14 Zeljan Rikalo LazTarget 1.0 => 1.2
2012-02-04 13:14 Zeljan Rikalo Note Added: 0056377
2012-02-05 01:40 Martin Friebe Note Added: 0056502
2012-02-05 12:39 Zeljan Rikalo Note Added: 0056509
2012-03-13 07:50 Vincent Snijders Target Version 1.0.0 => 1.2.0
2014-01-14 15:10 Martin Friebe LazTarget 1.2 => 1.4
2014-01-14 15:12 Martin Friebe Target Version 1.2.0 => 1.4
2015-01-31 16:50 Bart Broersma Note Added: 0080700
2015-01-31 16:59 Bart Broersma Summary SetCursor faulty under GTK2 => chnages cursor for wrong control => SetCursor faulty under GTK2 => changes cursor for wrong control
2015-02-13 11:39 G. Colla Note Added: 0081023
2015-02-13 14:24 Bart Broersma Note Added: 0081026
2015-02-13 23:52 G. Colla Note Added: 0081048
2015-02-16 00:50 G. Colla Note Added: 0081121
2015-02-16 00:50 G. Colla Status acknowledged => resolved
2015-02-16 00:50 G. Colla Resolution open => fixed
2015-02-16 00:50 G. Colla Assigned To => G. Colla
2015-02-17 00:44 G. Colla Note Added: 0081155
2015-02-17 00:44 G. Colla Status resolved => assigned
2015-02-17 00:44 G. Colla Resolution fixed => reopened
2015-02-17 00:45 G. Colla Note Added: 0081156
2015-02-17 00:45 G. Colla Status assigned => resolved
2015-02-17 00:45 G. Colla Resolution reopened => fixed
2015-02-17 07:53 Zeljan Rikalo Note Added: 0081159
2015-02-17 08:33 Zeljan Rikalo Fixed in Revision => 47841
2015-02-17 08:33 Zeljan Rikalo Note Added: 0081160
2015-02-17 09:46 G. Colla Note Added: 0081161
2019-04-22 19:33 Martin Friebe Status resolved => closed