Created attachment 182264 [details] [review]
Normalise marks positions internally in GtkScale

This means that marks added to the top will go left when
the orientation is changed. This also means that we can
rely on the position of the marks regardless of orientation.

Created attachment 182265 [details] [review]
Add style classes to scales when they have marks

So that theme engines can draw their sliders in more meaningful
ways when marks are used, or marks are not present.

Review of attachment 182264 [details] [review]:

Looks like a reasonable idea, but you will have to fix some more places where mark->position is currently compared. 
Eg. the calls to find_next_pos (...GTK_POS_LEFT...)

::: gtk/gtkscale.c
@@ +1535,3 @@
+    mark->position = GTK_POS_TOP;
+  else
+    mark->position = GTK_POS_BOTTOM;

This contradicts the gtk_scale_add_mark docs:

* @position: where to draw the mark. For a horizontal scale, #GTK_POS_TOP
 *   is drawn above the scale, anything else below. For a vertical scale,
 *   #GTK_POS_LEFT is drawn to the left of the scale, anything else to the
 *   right.

Comment on attachment 182265 [details] [review]
Add style classes to scales when they have marks


> 
>+  /* Set the style classes for the slider, so it could
>+   * point to the right direction when marks are present */

Pet peeve of mine: move the closing */ to the next line and line
up the stars

>+  context = gtk_widget_get_style_context (GTK_WIDGET (scale));
>+  if (all_pos != GTK_POS_MULTIPLE &&
>+      all_pos != GTK_POS_UNSET)
>+    {
>+      if (all_pos == GTK_POS_TOP)
>+        gtk_style_context_add_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_ABOVE);
>+      else if (all_pos == GTK_POS_BOTTOM)
>+        gtk_style_context_add_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_BELOW);
>+    }
>+  else
>+    {
>+      gtk_style_context_remove_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_BELOW);
>+      gtk_style_context_remove_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_ABOVE);
>+    }

Shouldn't this be a straight if-else cascade a la:

remove_all_classes()
if (all_pos == GTK_POS_MULTIPLE)
  add_class (GTK_STYLE_CLASS_MARK)
else if (all_class == GTK_POS_TOP)
  add_class (GTK_STYLE_CLASS_HAS_MARK_ABOVE)
else if (all_class == GTK_POS_BOTTOM)
  add_class (GTK_STYLE_CLASS_HAS_MARK_BELOW)

?

Created attachment 182337 [details] [review]
Add style classes to scales when they have marks

So that theme engines can draw their sliders in more meaningful
ways when marks are used, or marks are not present.

(In reply to comment #4)
> (From update of attachment 182265 [details] [review])
> 
> > 
> >+  /* Set the style classes for the slider, so it could
> >+   * point to the right direction when marks are present */
> 
> Pet peeve of mine: move the closing */ to the next line and line
> up the stars

Done.

> >+  context = gtk_widget_get_style_context (GTK_WIDGET (scale));
> >+  if (all_pos != GTK_POS_MULTIPLE &&
> >+      all_pos != GTK_POS_UNSET)
> >+    {
> >+      if (all_pos == GTK_POS_TOP)
> >+        gtk_style_context_add_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_ABOVE);
> >+      else if (all_pos == GTK_POS_BOTTOM)
> >+        gtk_style_context_add_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_BELOW);
> >+    }
> >+  else
> >+    {
> >+      gtk_style_context_remove_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_BELOW);
> >+      gtk_style_context_remove_class (context, GTK_STYLE_CLASS_SCALE_HAS_MARKS_ABOVE);
> >+    }
> 
> Shouldn't this be a straight if-else cascade a la:

Simplified. Didn't know whether adding/removing classes had a possible speed impact.

> remove_all_classes()
> if (all_pos == GTK_POS_MULTIPLE)
>   add_class (GTK_STYLE_CLASS_MARK)

GTK_STYLE_CLASS_MARK is only added to the style context when the marks will actually be drawn in _draw() (and is then restored without that class).

Comment on attachment 182337 [details] [review]
Add style classes to scales when they have marks

The approach looks good to me. For completeness, I think that gtk_scale_add_mark() should handle GTK_POS_MULTIPLE, adding both classes in that case.

(In reply to comment #7)
> (From update of attachment 182337 [details] [review])
> The approach looks good to me. For completeness, I think that
> gtk_scale_add_mark() should handle GTK_POS_MULTIPLE, adding both classes in
> that case.

What would this be useful for though?

Doubly pointy sliders? :), just for completeness as I said, it makes sense to me that the slider is able to accurately pinpoint the position on both sides if there are marks there and the engine wants so.