View Issue Details

IDProjectCategoryView StatusLast Update
0032392LazarusLCLpublic2018-12-18 22:08
ReporterAlexanderAssigned ToMichl 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformAnyOSWinOS VersionAny
Product VersionProduct Build 
Target Version1.8Fixed in Version1.9 (SVN) 
Summary0032392: Incorrect mouse position for TScrollingWinControl during DragNDrop
DescriptionOther controls accept incorrect local mouse coordinates if source dragged control was scrolled.
Steps To Reproduce1. Open attached sample
2. Scroll down a little TScrollBox on the left
3. Drag this TScrollBox into TForm client area
4. Look at caption text which displays client coordinates from FormDragOver event
These coordinates have a shift from the source control scroll position.

If you have a scroll too much then dragndrop event not produced at all.
Additional InformationI investigated this behavior and found a pretty simple solution.
Open
win32winapi.inc
file and find
function TWin32WidgetSet.ClientToScreen(Handle: HWND; var P: TPoint): Boolean;
method.
This method contains shift at the end:
  inc(P.X, ORect.Left);
  inc(P.Y, ORect.Top);
If you remove this lines (or comment it) drag-drop works as expected.
I tried to understand why this shift is required here but actually has no any ideas.
TagsNo tags attached.
Fixed in Revision55833, 55857
LazTarget-
Widgetset
Attached Files

Relationships

related to 0012217 resolvedZeljan Rikalo Tscrollbox doesn't report the correct X,Y value in event onmousemove 
related to 0018272 closedZeljan Rikalo Mouseenter dont occur in a control, which is in scrollbox when scrolled 
related to 0034714 resolvedMichl TGraphicControl.Cursor not working properly with TScrollBox 

Activities

Alexander

2017-09-10 03:01

reporter  

DragProblem.zip (2,145 bytes)

jamie philbrook

2017-09-10 17:37

reporter   ~0102755

Last edited: 2017-09-10 17:39

View 2 revisions

That is what suppose to happen.

 You are dragging from the client which has an offset due to the
scrollbar not being at positon 0.

  if you were to have a button sitting on the client out of view the
client would require you to scroll to see it. Your fix would break
it because it would not correctly report your button location.
 
 If you need only the client area which isn't always the absolute X,Y mouse
location, you need to subtract the offsets of the scrollbars.

EDIT:
 Not only would it break this, it would break a lot of stuff.

Alexander

2017-09-10 21:00

reporter   ~0102760

> That is what suppose to happen.
No. I receive wrong client coordinates of the target location.

This is what happens in Lazarus:
https://habrastorage.org/web/51b/d82/390/51bd82390afe4d89abd8b8483f609595.gif

This is correct behavior (in Delphi for example):
https://habrastorage.org/web/97a/1be/5ad/97a1be5ad0e8415f9ee57a6c22e93fdd.gif

As you can see DragNDrop in Lazarus totally broken for scrolled controls.

> Not only would it break this, it would break a lot of stuff.
Yes, probably my solution wrong, but it's not a patch. I describe it only for future investigation. It's a real bug and requires fix it.

Michl

2017-09-10 21:49

developer   ~0102761

Last edited: 2017-09-10 21:51

View 3 revisions

There is a bug. I'm working on a fix for it. ScrollingWinControl.ClientToScreen and some other methods need to be fixed. The current behaviour isn't Delphi compatible.

This bug isn't a Widgetset bug. Win32, Gtk2, Qt show all the same bug.

I've fixed the issue local here. But it needs a lot of testing, as it break code.

Please be patient, it will take a while.

Michl

2017-09-10 23:23

developer   ~0102765

I can't see any sideeffects, so I fixed ScreenToClient and ClientToScreen for TScrollingWinControl. Now it is Delphi Berlin compatible.

Please test real world GUI apps, if it breaks code.

Fixed in Lazarus trunk revision 55833.

jamie philbrook

2017-09-10 23:28

reporter   ~0102766

I hope you only fixed it in the drag operations? Because what I found that it
was using the offset for the source on the target, when the target should be
using it's own offset.

Michl

2017-09-10 23:40

developer   ~0102767

Last edited: 2017-09-10 23:41

View 2 revisions

No, ClientToScreen and ScreenToClient were calculated wrong (not Delphi compatible).

Please test and report possible problems!

Alexander

2017-09-11 08:04

reporter   ~0102772

Last edited: 2017-09-11 08:06

View 2 revisions

Jamie Philbrook was right.
We can't to fix ClientToScreen and ScreenToClient and should fix just a DragNDrop case. As far as I see offset from source affect a target. Jamie Philbrook was right twice.
Now current fix in revision 55833 broke a lot of stuff (according to my tests). For example, it broke hit test on mouse move and hint system for scrolled controls.
For example, this bug comes after fix:
https://habrastorage.org/web/2ab/cd9/ceb/2abcd9ceb6ea4790a39cc45d443215c3.gif

Thank you, Michl, for your fix, but it's broke other things.
Thank you, Jamie, for your inflexibility.
I'm sorry that my haste led to this fix.
We need investigate reported bug deeper before the fix, and estimate all risks with a cold mind.

Michl

2017-09-11 10:04

developer   ~0102773

Last edited: 2017-09-11 11:45

View 2 revisions

No, this wasn't a fast shot. ClientToScreen and ScreenToClient are calculated Delphi incompatible. The wrong DragNDrop coordinates is a result of it.

We have two choices:
- make it Delphi compatible
- document the incompatibility

Now we can test the first choice in Trunk. If nothing real wild speaks against it, this is the way to go.

Please report problems and add minimal examples to show it.

Thank you

Michl

2017-09-11 10:13

developer  

HintTest.zip (2,304 bytes)

Michl

2017-09-11 10:21

developer   ~0102774

> Now current fix in revision 55833 broke a lot of stuff (according to my tests).
> For example, it broke hit test on mouse move and hint system for scrolled
> controls.
> For example, this bug comes after fix:

Please add a minimal example. I tested the hints yesterday and couldn't see problems on Win32, Gtk2, Qt. You can also test the added HintTest.zip.

Alexander

2017-09-12 02:47

reporter  

HintTest_withGraphic.zip (2,469 bytes)

Alexander

2017-09-12 02:53

reporter   ~0102790

HintTest_withGraphic.zip added.
Your test with TButton doesn't work because it inherited from TWinControl. Windows go bypassing ClientToScreen at this case. You can put any control inherited from TGraphicControl for checking bug. I used TSpeedButton and attached to archive.

Alexander

2017-09-12 03:02

reporter   ~0102791

> We have two choices:
> - make it Delphi compatible
It's is best scenario, but it can broke a lot of other custom user controls.

> - document the incompatibility
It's bad scenario. All custom controls can require to have two implementations with ifdefs in Delphi and Lazarus. But it solution what can help save backward compatibility for a lot 3rd party controls libraries.

I can not make this choice. Too much responsibility for me, who never contributed to lazarus/fpc yet.

Zeljan Rikalo

2017-09-13 20:45

developer   ~0102830

I can confirm that hints does not work with this commit (HintTest_withGraphic.zip example) under Qt4, Qt5,gtk2 and gtk3 under fedora 25 64bit.

Michl

2017-09-13 21:33

developer   ~0102831

Thank you for testing!

I fixed ControlAtPos for TScrollingWinControl in revision 55857. The example should work now.

If there are more problems, please report!

Zeljan Rikalo

2017-09-18 12:45

developer   ~0102904

Looks ok now - tested qt and qt5.

Issue History

Date Modified Username Field Change
2017-09-10 03:01 Alexander New Issue
2017-09-10 03:01 Alexander File Added: DragProblem.zip
2017-09-10 08:41 Michael Van Canneyt Project FPC => Lazarus
2017-09-10 13:17 Michl Assigned To => Michl
2017-09-10 13:17 Michl Status new => assigned
2017-09-10 17:37 jamie philbrook Note Added: 0102755
2017-09-10 17:39 jamie philbrook Note Edited: 0102755 View Revisions
2017-09-10 21:00 Alexander Note Added: 0102760
2017-09-10 21:49 Michl Note Added: 0102761
2017-09-10 21:50 Michl Note Edited: 0102761 View Revisions
2017-09-10 21:51 Michl Note Edited: 0102761 View Revisions
2017-09-10 23:16 Michl File Added: test.zip
2017-09-10 23:23 Michl LazTarget => -
2017-09-10 23:23 Michl Note Added: 0102765
2017-09-10 23:23 Michl Status assigned => feedback
2017-09-10 23:24 Michl Fixed in Revision => r55833
2017-09-10 23:24 Michl Product Version 3.0.2 =>
2017-09-10 23:24 Michl Fixed in Version => 1.9 (SVN)
2017-09-10 23:25 Michl Target Version => 1.8
2017-09-10 23:28 jamie philbrook Note Added: 0102766
2017-09-10 23:40 Michl Note Added: 0102767
2017-09-10 23:41 Michl Note Edited: 0102767 View Revisions
2017-09-11 08:04 Alexander Note Added: 0102772
2017-09-11 08:04 Alexander Status feedback => assigned
2017-09-11 08:06 Alexander Note Edited: 0102772 View Revisions
2017-09-11 10:04 Michl Note Added: 0102773
2017-09-11 10:12 Michl File Deleted: test.zip
2017-09-11 10:13 Michl File Added: HintTest.zip
2017-09-11 10:21 Michl Note Added: 0102774
2017-09-11 10:24 Michl Status assigned => feedback
2017-09-11 11:45 Michl Note Edited: 0102773 View Revisions
2017-09-12 02:47 Alexander File Added: HintTest_withGraphic.zip
2017-09-12 02:53 Alexander Note Added: 0102790
2017-09-12 02:53 Alexander Status feedback => assigned
2017-09-12 03:02 Alexander Note Added: 0102791
2017-09-13 09:14 Zeljan Rikalo Relationship added related to 0012217
2017-09-13 09:14 Zeljan Rikalo Relationship added related to 0018272
2017-09-13 20:45 Zeljan Rikalo Note Added: 0102830
2017-09-13 21:33 Michl Note Added: 0102831
2017-09-13 21:33 Michl Status assigned => feedback
2017-09-13 21:33 Michl Fixed in Revision r55833 => 55833, 55857
2017-09-18 12:45 Zeljan Rikalo Note Added: 0102904
2017-09-19 22:25 Michl Status feedback => resolved
2017-09-19 22:25 Michl Resolution open => fixed
2018-12-18 22:08 Michl Relationship added related to 0034714