[haiku-development] Re: BControlLook scrollbar APIs

  • From: John Scipione <jscipione@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Sun, 22 Jul 2018 11:55:54 -0700

On Sun, Jul 22, 2018, 12:15 AM Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
wrote:

The API for BControlLook was changed recently. Instead of the old
piece-wise APIs for drawing the scrollbar background and buttons, there
is now a single DrawScrollbar call.


There are 2 calls: DrawScrollBar and DrawScrollBarThumb. One draws the
border and arrow buttons and the other draws the thumb and thumb
background... the goal was to do as much drawing as possible in
HaikuControlLook.

Unfortunately, this is a problem for WebPositive, because it manages its
own scrollbar logic. So it needs to know where the scrollbar buttons are,
to do the right think when it gets a mouse click event in the scrollbar.


Yeah I saw this (although not fast enough because I'm still not able to
cross-compile x86 just x86_gcc2). Looking at a nightly the scroll bars do
not repaint themselves correctly in WebPositive anymore.

BScrollBar has its own _ButtonFor code, but in the current state this
would not work with a replacement BControlLook with different button
sizes (or no buttons at all, or…). The code in _ButtonFor would get out
of sync with the one in BControlLook.


You're right that the arrow button positioning code currently happens in
both BScrollBar and HaikuControlLook and they have to match.

I now see where in the code in HaikuWebkit does its drawing in
https://github.com/haiku/webkit/blob/rebased/Source/WebCore/platform/haiku/ScrollbarThemeHaiku.cpp
and it needs to implement the following methods:

backButtonRect
forwardButtonRect
trackRect
paintScrollCorner
paintScrollbarBackground
paintButton
paintThumb

The first 3 reproduce code in BScrollBar (sharing code would be better) and
then the latter 4 should call into BControlLook.

We have two options here:
- Either revert to separate DrawButtons and DrawBackground methods, and
  let BScrollBar manage the layout. In this case, BControlLook cannot
  change the button layout of the scrollbar
- Or, let BControlLook tell BScrollBar where the buttons should be
  placed.


I created the following patchset to try and address these WebPositive
drawing issues in BControlLook and BScrollBar:
https://review.haiku-os.org/#/c/haiku/+/348/

The patch changes the BControlLook API from DrawScrollBar and
DrawScrollBarThumb to DrawScrollBackground and DrawScrollBarThumb. The big
difference is that all of the arrow positioning happens in BScrollBar again
instead of in BControlLook and then BScrollBar calls BControlLook
DrawButtonBackground and DrawButtonShape to draw arrow buttons but
otherwise BScrollBar is once again solely responsible for setting up the
arrow button positions and frame rectangles. Webpositive can (and does) do
the same. DrawScrollbarBackground is used to draw the non-thumb area, and
then DrawScrollBarThumb draws the thumb. Unfortunately we have to draw the
border in BScrollBar again which kills proper theming support for a
different looking border. Webpositive will need to (and does) reproduce the
button positioning code from BScrollBar.

The DrawScrollBarBackground method was the one that Webpositive was using a
lot so getting that back will help WebPositive to draw correctly again
without any further HaikuWebKit code changes. We should however update
paintThumb to call DrawScrollBarThumb so that we get knob styles in
WebPositive.

Sorry for the late comments, I had no time to review the BControlLook
version of https://review.haiku-os.org/#/c/haiku/+/300/ which spent just
6 hours on review before being +2 and merged by the same person (I would
prefer we avoid that, unless there is an emergency).


We could have used this input a little earlier to avoid this but let's move
forward and fix it up.

I will have to delay fixes to WebKit until a decision is made and the
new solution implemented. And meanwhile, I can't build WebKit because
the API has changed and the new one is not usable.


This is preventing Beta right now so let's try to get this back up and
working soon. This seeks to fix up the problem without extending
BControlLook and breaking the API. However a better API breaking fix would
reproduce all of the scroll bar drawing that WebPositive needs in
BControlLook without having to reproduce code from BScrollBar.

Other related posts: