After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749910 - gtkdnd: draw highlight before default draw handler
gtkdnd: draw highlight before default draw handler
Status: RESOLVED WONTFIX
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-05-26 14:58 UTC by Carlos Soriano
Modified: 2015-06-14 16:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtkdnd: draw highlight before default draw handler (981 bytes, patch)
2015-05-26 14:58 UTC, Carlos Soriano
needs-work Details | Review
gtkdnd: use css class for drag highlight (163.22 KB, patch)
2015-05-28 15:27 UTC, Carlos Soriano
none Details | Review

Description Carlos Soriano 2015-05-26 14:58:42 UTC
The problem is that the gdk_drag_highligth doesn't work at all when used with
gtk_list_box_drag_highligth_row, after debugging I foudn that the draw signal
of the widget is not emited so the draw_highlight callback is not called.
Connecting to the draw signal by just g_signal_connect instead of
g_signal_connect_after fixes it.

I'm sure this is not a great fix and it's actually hidding a deeper issue,
since the draw signal should be emitted anyway, but couldn't figure where all
the draw is done.

Attached a patch that "fixes" it for debuggin pourposes.
Comment 1 Carlos Soriano 2015-05-26 14:58:46 UTC
Created attachment 304020 [details] [review]
gtkdnd: draw highlight before default draw handler

Seems connecting after the default draw handler makes the widget
not sending the draw signal.
Comment 2 Matthias Clasen 2015-05-26 17:30:26 UTC
Review of attachment 304020 [details] [review]:

if you draw the highlight first, it will (in general) be covered up by the drawing of the widget, so this can't work as-is.
Comment 3 Carlos Soriano 2015-05-27 09:18:07 UTC
(In reply to Matthias Clasen from comment #2)
> Review of attachment 304020 [details] [review] [review]:
> 
> if you draw the highlight first, it will (in general) be covered up by the
> drawing of the widget, so this can't work as-is.

I was expecting the same, it's just I don't understand where to look more to fix the real issue. So this was more a bug report with a testcase that workaround it (badly), but not an actual fix.

Do you have idea where should I look deeper?
Comment 4 Carlos Garnacho 2015-05-27 10:09:00 UTC
Rough guess, probably boils down to draw handlers returning TRUE, thus blocking further propagation, note the "draw" signal has _gtk_boolean_handled_accumulator accum function.

I'll suggest a completely different approach, let's just add/remove a css style class and deal with that from css, this way we have something nicer than a black rectangle there.
Comment 5 Matthias Clasen 2015-05-27 10:20:27 UTC
(In reply to Carlos Garnacho from comment #4)

> I'll suggest a completely different approach, let's just add/remove a css
> style class and deal with that from css, this way we have something nicer
> than a black rectangle there.

Yeah, right. I think we may even have a bug for that somewhere.
Comment 6 Carlos Soriano 2015-05-27 11:14:08 UTC
(In reply to Carlos Garnacho from comment #4)
> Rough guess, probably boils down to draw handlers returning TRUE, thus
> blocking further propagation, note the "draw" signal has
> _gtk_boolean_handled_accumulator accum function.
> 
> I'll suggest a completely different approach, let's just add/remove a css
> style class and deal with that from css, this way we have something nicer
> than a black rectangle there.

Thanks for your answer. I tried to search for the actual drawing function of the widget sending the draw signal, but couldn't find it (since I guessed the problem was that the signal was handled somewhere).

As far as I understood, it's actually adding a CSS style (along with drawing a rectangle with cairo) in gtk_drag_highlight_draw when the draw signal is triggered, so I didn't understand completely your proposal.
Comment 7 Carlos Garnacho 2015-05-27 13:53:11 UTC
(In reply to Carlos Soriano from comment #6)
> As far as I understood, it's actually adding a CSS style (along with drawing
> a rectangle with cairo) in gtk_drag_highlight_draw when the draw signal is
> triggered, so I didn't understand completely your proposal.

What I mean is doing:

static void
gtk_drag_highlight (GtkWidget *widget)
{
  gtk_style_context_add_class (gtk_widget_get_style_context (widget),
                               "dnd-target");
}

static void
gtk_drag_unhighlight (GtkWidget *widget)
{
  gtk_style_context_remove_class (gtk_widget_get_style_context (widget),
                                  "dnd-target");
}

i.e. the style class is "permanent" on the widget style context (as long as DnD lasts), instead of doing an extraneous gtk_render_frame(), and you can have

*.dnd-target { border: 1px solid #000; }

in your local css for testing. We might even do highlighting by default and make something nice looking for some selected widgets.
Comment 8 Carlos Soriano 2015-05-27 14:38:15 UTC
Ah okay, I though gtk_style_context_add_class (context, GTK_STYLE_CLASS_DND) was doing something similar.

I will take a look to implement it (because I guess it's not as easiest as your code), thanks Carlos!
Comment 9 Matthias Clasen 2015-05-27 19:48:01 UTC
> Ah okay, I though gtk_style_context_add_class (context, GTK_STYLE_CLASS_DND) 
> was doing something similar.

It does.

But as Carlos says, it relies on an extra render_frame call. I'm not 100% it will work to just rely on any widgets draw() function to do something meaningful with the extra style class, but we should try.
Comment 10 Carlos Soriano 2015-05-28 15:27:12 UTC
Created attachment 304189 [details] [review]
gtkdnd: use css class for drag highlight

Instead of a temporary style class and a caire rectangle around
the widget.

The only drawback about this is that GtkTreeView applies all the
style to the widget itself to its cells, so when gtk_drag_highlight
is applied, the cells show borders.
To prevent that, remove the dnd style class from the draw function
on GtkTreeView. The drawback about this solution is that GtkTreeView
won't draw a border to indicate "general" drop target availability, but
individual cells will still work as expect for highlight since they are
drawn in an internal way in GtkTreeView.
Comment 11 Matthias Clasen 2015-06-14 16:01:33 UTC
I don't think this is needed anymore ?