343987 - [BeOS] Rendering context and other gfx rework

Status: RESOLVED INVALID

Description

[Sergei Dolgov]

1)Implement line styles (easy)
2)Investigate why there are problems if we use non-cached backbuffer
3)There were several methods added to nsI(gfx) since last time I did here big revision.
e.g. GetRange and other text block methods, GetNativeGraphicsData in RenderingContext (requires also new constant for BeOS in nsIRenderingContext).

Will look for more things we missed.

Comment 1

[Sergei Dolgov]

Created attachment 228584 [details] [diff] [review]
patch

First portion of changes.
Most innocent - 
1)adds missing implementation of line styles (still didn't apply that for ::Fill*() methods - as I'm unsure if mozilla expects some line style in filling)
2)Fixes signed-unsigned warning
3)Adds in outcommented form two methods for deleting/creating backbuffer instead permanent one. That's will be topic for following patches - put to work it properly.

Comment 2

[tqh]

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

r=[e-mail]
AFAIK it looks good, I wonder if the consts shouldn't maybe be all uppercase, what's the codestyle for that here?

Comment 3

[Sergei Dolgov]

Checking in mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp;
/cvsroot/mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp,v  <--  nsRenderingContextBeOS.cpp
new revision: 1.60; previous revision: 1.59
done
Checking in mozilla/gfx/src/beos/nsRenderingContextBeOS.h;
/cvsroot/mozilla/gfx/src/beos/nsRenderingContextBeOS.h,v  <--  nsRenderingContextBeOS.h
new revision: 1.31; previous revision: 1.30
done

Comment 4

[Sergei Dolgov]

In regard of printing transition to widget we need either to move there nsDeviceContext - like MacOS idi, or fix Makefile.in (add 	-I$(srcdir)/../../../widget/src/beos \ in INCLUDES section) and nsDeviceContextBeOS.h (add there #include nsDeviceContextSpecB.h )

Comment 5

[Sergei Dolgov]

nsRenderingContextBeOS::CopyOffScreenBits()
in case of bitmapped view to bitmapped view blit works very well and fast even with simplest less than effective byte-by byte copy code:
PRBool offscreen;
destsurf->IsOffscreen(&offscreen);
if (offscreen)
{
BBitmap *dstbitmap = destsurf->GetBitmap();
uint32 *src = (uint32 *)srcbitmap->Bits() + srcX + srcY*srcbitmap->BytesPerRow();
uint32 *dst = (uint32 *)dstbitmap->Bits() + drect.x +  drect.height*dstbitmap->BytesPerRow();
for(uint j = 0; j < drect.height; j++)
for(uint i = 0; i < drect.width; i++)
dst[i + j*dstbitmap->BytesPerRow()] = src[i + j*srcbitmap->BytesPerRow()];
		
destview->Sync();
}
needs addition of GetBitmap() method to nsDrawingSurfaceBeOS.

Writing it now from SeaMonkey with such code.

Comment 6

[Sergei Dolgov]

Per comment #5:

Seems i was too fast for that report. Looks like bitmap-to-bitmap case is never happening, which means that we lack proxied buffering with separate bitmap proxy for each view and common backbuffer.

Situation needs investigation, and may have same reason as our inability to add-remove backbuffer permanently instead using cached.

Visible speedup I enjoyed in report may be caused by changes in nsWindow::OnPaint - rendering context and surface new interfaces i tried here.

Comment 7

[Sergei Dolgov]

I catched part of problem, why we have troubles in painting if we create/destroy backbuffer at each paint, as Windows and Linux do.

CreateDrawingSurface supplies in such case aBounds which reflects size of rect to update/repaint, while cached backbuffer always had size of initial BView.
Left and Top of such aBounds are always 0, 0.

Then we create BView inside that little clipped surface, attach it to little bitmap and drawing there.
Probably something here goes wrong with coordinates - like layout/rendering context supplies onscreen BView-related coordinates, while we need mSurface related ones, when calling painting primitives - or ::CopyOffScreenBits does things wrong for some reason

Comment 8

[Sergei Dolgov]

Well, problem is in nsRenderingContextBeOS::CopyOffScreenBits()
if (aCopyFlags & NS_COPYBITS_USE_SOURCE_CLIP_REGION) 
{
srcview->GetClippingRegion(&r);
destview->ConstrainClippingRegion(&r);
}
code.

In case of little temporary backbuffers which are "placed" over arbitrary dirty rects of onscreenvew, we cannot get that bacbuffers-view clip region and put it
 as big onscreenview clip-region.

But if we ignore that flag, painting, inspite it works now in both cases of permanent or temporaty backbuffers, goes messy and stochastic.

So, our clipping region support is crap atm. And we do it in several places in widget and gfx.

Comment 9

[Sergei Dolgov]

Created attachment 228715 [details] [diff] [review]
patch

Sets proper clipping for CopyOffscreenBits.
Allows to use temporary backbuffers, which reduces RAM-consumption, but should be estimated with some tests in regard of drawing speed.
Also, I hope, allows to use native BView::Scroll() in future.

r=?

Comment 10

[tqh]

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

r=[e-mail]
Looks ok AFAIK.

Comment 11

[Sergei Dolgov]

Checking in mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp;
/cvsroot/mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp,v  <--  nsRenderingContextBeOS.cpp
new revision: 1.61; previous revision: 1.60
done 

Comment 12

[Sergei Dolgov]

According to comment in OS2 code
http://lxr.mozilla.org/seamonkey/source/gfx/src/os2/nsRenderingContextOS2.cpp#524
looks like 0-sized regions are possible and actually in use to draw nothing.

Wondering if it is better just return from drawing primitive in beginning if one of region frame dimensions is 0, instead drawing it with clipping.

In our case it cab be easily achieved

Comment 13

[Sergei Dolgov]

Created attachment 229898 [details] [diff] [review]
patch

Patch. Changes a bit LockAndUpdateView() code.
If there is mClipRegion and if some size  of native region frame is less-equal 0,
it returns false immediatelly.
It allows us to avoid any calculation and app_server poking in all drawing primitives, as we use that method everywhere we draw.

Comment 14

[Sergei Dolgov]

CopyOffscreenBits fix and maybe other changes need backporting to branch.

Comment 15

[Sergei Dolgov]

Created attachment 288839 [details] [diff] [review]
patch for 1.8 branch

two first patches together adapted to branch.
r=?

Comment 16

[Sergei Dolgov]

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

In current form this 3rd patch when applied creates drawing artifacts, if you scroll pages with pressed middle-button drag.
So obsoleting.

Comment 17

[Doug Shelton]

Comment on attachment 288839 [details] [diff] [review]
patch for 1.8 branch

reviewed, applied against branch and tested.  We've missed the 1.8.1.10 cutoff, so requesting approval for 1.8.1.11.

Comment 18

[Samuel Sidler (old account; do not CC)]

Comment on attachment 288839 [details] [diff] [review]
patch for 1.8 branch

Approved for 1.8.1.12, a=ss for release-drivers.

Comment 19

[Sergei Dolgov]

Checking in mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp;
/cvsroot/mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp,v  <--  nsRenderingContextBeOS.cpp
new revision: 1.51.12.8; previous revision: 1.51.12.7
done
Checking in mozilla/gfx/src/beos/nsRenderingContextBeOS.h;
/cvsroot/mozilla/gfx/src/beos/nsRenderingContextBeOS.h,v  <--  nsRenderingContextBeOS.h
new revision: 1.28.12.3; previous revision: 1.28.12.2
done 

Comment 20

[Joe Drew (not getting mail)]

We don't support BeOS any more.

Comment 21

[JP Rosevear [:jpr]]

In the graveyard, code referred to doesn't exist to be fixed.