Add uiDragDestination for text and file drop. #245
Conversation
szanni
force-pushed
the
control-drag-destination
branch
from
94e6a39
to
f194eb0
Compare
New types: uiDragType uiDragOperation New structs: uiDragDestination uiDragContext uiDragData WARNING: Currently only works cross platform for uiWindow.
- uiBox - uiButton - uiCheckbox - uiColorButton - uiCombobox - uiDateTimePicker - uiEditableCombobox - uiFontButton - uiForm - uiGrid - uiMultilineEntry - uiSlider - uiSpinbox - uiTable MISSING: - uiEntry - uiGroup - uiLabel - uiProgressBar - uiRadioButtons - uiSeperator - uiTab
| * | ||
| * @memberof uiDragContext | ||
| */ | ||
| _UI_EXTERN void uiDragContextPosition(uiDragContext *dc, int *x, int *y); |
There was a problem hiding this comment.
Possibly rename this to uiDragContextCursorPosition?
test/qa/meson.build
Outdated
There was a problem hiding this comment.
I noticed msvc requires /execution-charset:utf-8 as a compiler option to use utf-8 literals.
libui_qa_c_args = []
if libui_OS == 'windows'
if libui_MSVC
libui_qa_c_args += ['/execution-charset:utf-8']
endif
endif
executable('qa', libui_qa_sources,
dependencies: libui_binary_deps,
link_with: libui_libui,
c_args: libui_qa_c_args,
gui_app: false,
install: false)Or msvc replaces utf-8 characters with "?" (at least on Windows 10 with Visual Studio 2022.)
There was a problem hiding this comment.
Thanks for catching that. I had not encountered this. Maybe because of Windows 7 or using only mingw compiled binaries for testing.
Edit: This should probably be it's own bug? Looks like Unicode string literals are broken on Windows 10/MSVC?
There was a problem hiding this comment.
I think the cause is the system locale. (Control Panel > Clock and Region > Region > Administrative > Change system locale)
Windows still uses old charsets for localization (even for the English locales), and MSVC uses the locale to parse files.
You can change the locale to UTF-8 but it's still beta.
|
I confirmed that the qa tests work fine on Ubuntu 20.04 and Windows 10. (edit) |
|
These are nits but codespell found more typos. And |
|
I noticed the qa test crashed at Here is the backtrace. (edit) |
| if (types & accepted & uiDragTypeText) { | ||
| data = uiDragContextDragData(dc, uiDragTypeText); | ||
| if (data != NULL) { | ||
| uiMultilineEntryAppend(dragData, "text:\n"); |
There was a problem hiding this comment.
Confirmed that data->type can be uiDragTypeURIs here on Ubuntu 22.04. I think it's related to the crash.
unix/control.c
Outdated
| d->data.URIs.URIs = uiprivAlloc(d->data.URIs.numURIs * sizeof(*d->data.URIs.URIs), "uiDragData->data.URIs.URIs"); | ||
|
|
||
| for (i = 0; uris[i] != NULL; ++i) { | ||
| d->data.URIs.URIs[i] = g_filename_from_uri(uris[i], NULL, NULL); |
There was a problem hiding this comment.
I also noticed I can get another kind of errors at 7.Drag Context Drag Data > Step 1 on Ubuntu 22.04.
(qa:7735): Gtk-CRITICAL **: 12:35:14.684: gtk_text_buffer_insert: assertion 'text != NULL' failed
Thread 1 "qa" received signal SIGSEGV, Segmentation fault.
0x00007ffff7dec47e in __GI___libc_free (mem=0x20) at ./malloc/malloc.c:3368
3368 ./malloc/malloc.c: No such file or directory.
#0 0x00007ffff7dec47e in __GI___libc_free (mem=0x20)
at ./malloc/malloc.c:3368
#1 0x00007ffff7f9f020 in uiFreeDragData (d=0x555555a36dd0)
at ../unix/dragdata.c:13
#2 0x00005555555582c9 in updateDragData
(dd=0x555555ada470, dc=0x7fffffffd8b0)
at ../test/qa/dragdestination.c:52
#3 0x0000555555558880 in onEnterData
(dd=0x555555ada470, dc=0x7fffffffd8b0, senderData=0x0)
at ../test/qa/dragdestination.c:324
#4 0x00007ffff7f9c047 in onDragMotion
(widget=0x55555563ea60, context=0x55555560d020, x=98, y=50, time=10454598, userdata=0x555555794350) at ../unix/control.c:41
#5 0x00007ffff7905c96 in () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#6 0x00007ffff7246700 in g_signal_emit_valist ()
at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#7 0x00007ffff7246a8e in g_signal_emit_by_name ()
at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#8 0x00007ffff78e937a in () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#9 0x00007ffff77709fb in gtk_main_do_event ()
at /lib/x86_64-linux-gnu/libgtk-3.so.0
So, d->data.URIs.URIs[i] can be NULL here.
Then, the qa test crashes at uiFreeDragData.
| priv->requestedType = type; | ||
| gtk_drag_get_data(dc->widget, dc->context, atom, dc->time); | ||
| while (!priv->dataReceived) | ||
| if (!uiMainStep(1)) |
There was a problem hiding this comment.
Confirmed that this uiMainStep can fire other drag-drop events.
I think that's the cause of all crashes on Ubuntu22.04.
I added printf functions to trace the function calls.
[onEnterData] fired
[updateDragData] fired
[uiDragContextDragData] Requested type: 1
[uiMainStep]
[onDragDataReceived] info: 1
[uiDragContextDragData] Received type: 1 (Requested type: 1)
[uiDragContextDragData] Requested type: 2
[uiMainStep]
[onMoveData] fired
[updateDragData] fired
[uiDragContextDragData] Requested type: 1
[uiMainStep]
[onMoveData] fired
[onMoveData] finished
[uiMainStep]
[onMoveData] fired
[onMoveData] finished
[uiMainStep]
[onMoveData] fired
[onMoveData] finished
[uiMainStep]
[onDragDataReceived] info: 1
[uiDragContextDragData] Received type: 1 (Requested type: 1)
[uiDragContextDragData] Requested type: 2
[uiMainStep]
[onDragDataReceived] info: 2
[uiDragContextDragData] Received type: 2 (Requested type: 2)
[updateDragData] finished
[onMoveData] finished
[uiDragContextDragData] Received type: 2 (Requested type: 2)
(qa:2764): Gtk-CRITICAL **: 05:34:23.269: gtk_text_buffer_insert: assertion 'text != NULL' failed
Segmentation fault (core dumped)
They showed that onMoveData was fired while requesting URIs in onEnterData.
Then, uiDragContextDragData returned broken priv->data for onEnterData.
There was a problem hiding this comment.
Hm... maybe a synchronous API is not possible after all? Not sure.
Maybe just adding another check within !priv->dragEnter would do the trick to block out all other processing? Seems very brittle though.
It does make sense that uiMainStep() would process all kinds of events... including mouse moves.
Can you reproduce this bug with the new patches? Because it shouldn't crash IMO, even if the order is all garbled.
And attach your gtk version as well?
There was a problem hiding this comment.
Can you reproduce this bug with the new patches?
I got the same crash as the first one.
The problem is uiDragContextDragData can return the same pointer to both onEnterData and onMoveData.
In my case, uiDragContextDragData requests uiDragTypeURIs in onMoveData while requesting uiDragTypeText in onEnterData.
After returning URIs to onMoveData, it also returns the same URIs to onEnterData even if it requested uiDragTypeText.
Then, it crashes when using data->data.text.
And attach your gtk version as well?
3.24.33-1ubuntu2 on Ubuntu 22.04.
I think it also requires low-end machines to reproduce the situation.
I got segfault with 2GB RAM and one CPU core but it worked fine with 4GB RAM and dual core.
There was a problem hiding this comment.
I see, thanks for the condensed analysis. I am unsure about how to fix this.
We would need to essentially cache all move events (and possibly leave & drop) when still waiting for our data. Otherwise we might end up firing move or other events while the enter one has not even completed yet.
Simply blocking all further event processing while waiting for data might break the enter > move > leave/drop chain otherwise?
I'm almost tempted to implement XDnD myself. The API looks a lot saner than what Gtk offers.
It is strange that this seems to happen under memory pressure only. Gtk is very brittle, so I'm sadly not surprised. Thanks for the info though, maybe I can reproduce the issue with a 2GB RAM VM.
|
Thank you so much for testing! I will need to go through the crashes one by one. Gtk is such a pain...
Fixed, thanks.
Yes, as required by the C99 standard. Apparently C++11 dropped that requirement and I am unsure what Objective-C has to say about that. |
I mean, they have more blank lines, not only the newlines that C requires. I think your code editor hides the newlines. You can see two And it looks like this on VScode. |
Interesting. Learnt something new today. Vim apparently adds a hidden newline to all files by default. Looking at a hex dump does indeed confirm that. Seems silly to not display it. I'll fix that. |
Add text and file drop for all uiControls on all platforms.
This introduces new types:
You can then register the drag destination with a uiControl:
uiControlRegisterDragDestination().The
Drag and dropdocumentation included should give you a good overview on the basic workings.I currently only test
uiLabelas a drag destination. All other controls should work but are untested by me (apart fromuiWindow).Any suggestions on how to go about testing all controls? Do a drag destination test for each control within the
qatester? Most controls are not tested within yet.I would personally like to merge the other
qatests first to make sure I didn't break anything while refactoring the darwin code.I believe this should also close/supersede old PR #378 as we already have
uiWindowOnContentSizeChanged(),uiWindowOnFocusChanged(),uiWindowOnClosing()implemented upstream and file drop support will be provided by this PR.Fixes #217 - at least the initial request. Custom mime types are not included in this PR, but this should be possible by adding a
uiDragDestinationSetAcceptedStringTypes()or similar without interfering with the predefined types API, that most people will probably use.QT for example provides a conversion utility to convert from custom mimes to built in types. To me that honestly seems somewhat brittle. My thought are:
Provide a predefined types API via
uiDragDestinationAcceptedTypes.Provide a mime type API via
uiDragDestinationAcceptedStringTypesand filter out the overlapping predefined types.Reviews welcome. This is a massive change set.