315542 - [BeOS]Improper native drawing in Scroll()

Status: RESOLVED FIXED

(Core Graveyard :: Widget: BeOS, defect)

Description

[Sergei Dolgov]

Areas which we don't update explicitly in scrolling (those which were covered by other windows) don't update properly while scrolling. Problem includes softscroll icon, IME window, Menus etc.

Reason - due way in which Mozilla handles scrolling, we should offset also our cached native drawing region. GTK version does it also for mozwidget updateArea, but we recreate it from 0 in our Scroll(), so only native version needs shifting.

Also, for faster update we can include that accummulated (and now shifted) nsViewBeOS::paintregion in manually calculated BRegion invalid; used in Scroll().
Last action may slow-down scrolling a bit, but honestly, i didn't notice the difference (if paintregion was shifted before adding it to invalid. if we don't shift it, slow-down is more noticeable).

Only question for me is if we should introduce some special method in nsViewBeOS for shifting (e.g. nsViewBeOS::Scroll(aDx, aDy) {paintregion.OffsetBy(aDx, aDy);} or use paintregion directly, as it is now declared as public. For convinience i prefer first. Pacth will follow soon.

Comment 1

[tqh]

Sounds ok by me.

Comment 2

[Sergei Dolgov]

I planned 3 sample solution to publish here, but it seems the 4th is coming.
Most radical painting change in last year or two.

Comment 3

[Sergei Dolgov]

Attachment: patch

simplest version. Fixes bug, so uncovered areas will be drawn properly.
But with lag.

Comment 4

[Sergei Dolgov]

Attachment: patch

Second version, bit more complex and bit more faster (but not enough).
Native paintregion added to invalid region calculated in ::Scroll()

Comment 5

[Sergei Dolgov]

Attachment: patch - 3rd version

Most complex version.
nsWindow::OnPaint() and nsViewBeOS::Validate() now have single argument of BRegion* type.
fJustValidated moved in GetPaintRegion().
nsWindow::Invalidate() group of methods now uses only native paintregion from nsViewBeOS, filling it via mView->Draw().
In case of synchronous invalidiation it calls nsWindow::Update().

nsWindow::Update() is rewritten. Now it does what it was intended to do - flush collected updates. At first step it flushes pending BView::Draws() if any by calling BWindow::UpdateIfNeeded(), then gets paintregion from nsViewBeOS and puts it to nsWindow::OnPaint(BRegion *).

As result, areas uncovered from other windows update in sycn with other content at scroll, also scroll button is much more in sync with scrolling now.

mUpdateArea is now used once, in OnPaint(), to form nsIRegion argument for Mozilla painting event.

Thus, OnPaint() is used in 3 places now - in nsWindow::Scroll(), called with calculated dirty region, in nsWindow::CallMethod() ONPAINT: case, and in nsWindow::Update().
As all methods now use nsViewBeOS::paintregion consistently, i think some other issues may gone too, despite in some cases drawing may be bit slower (e.g. in during scroll it updates now not only page content, but XUL scroll-button in sync)

Comment 6

[Sergei Dolgov]

Comment on attachment 202284 [details] [diff] [review]
patch

review request.

More complicated paint change will form separate bug later

Comment 7

[tqh]

Comment on attachment 202284 [details] [diff] [review]
patch

r=[e-mail]

Comment 8

[Sergei Dolgov]

patch landed in trunk:
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.109; previous revision: 1.108
done

Closing bug

Comment 9

[Niels Reedijk]

Comment on attachment 202284 [details] [diff] [review]
patch

Requesting permission for landing on the MOZILLA_1_8_BRANCH. 

This is a BeOS-only change and does not affect other platforms.

This is the first patch in the new backporting queue

Comment 10

[Sergei Dolgov]

Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.91.4.17; previous revision: 1.91.4.16
done