Skip to content

Revert "HaikuCompositor: use opaque region" #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kenz-gelsoft
Copy link
Contributor

Reverting 3e61d9b fixes #20

I wonder why that change affects redrawing only when CPU is busy.

@X547
Copy link
Owner

X547 commented Dec 19, 2024

Does it help if put viewLocked->ConstrainClippingRegion(NULL); before drawing? It may be Haiku bug.

@kenz-gelsoft
Copy link
Contributor Author

Thanks, give it a try!

@kenz-gelsoft
Copy link
Contributor Author

This didn't help

~/src/haikuports/dev-libs/wayland-server/work-0.1.20241217/sources/wayland-server-c7f42756cd10868fc9ca24e6a34bf2c05472d37c> git diff
diff --git a/HaikuCompositor.cpp b/HaikuCompositor.cpp
index 2c1daf3..5edd173 100644
--- a/HaikuCompositor.cpp
+++ b/HaikuCompositor.cpp
@@ -223,6 +223,7 @@ void WaylandView::Draw(BRect dirty)
                if (mode == B_OP_ALPHA && fSurface->fState.opaqueRgn.has_value()) {
                        viewLocked->ConstrainClippingRegion(&fSurface->fState.opaqueRgn.value());
                        viewLocked->SetDrawingMode(B_OP_COPY);
+                       viewLocked->ConstrainClippingRegion(NULL);
                        viewLocked->DrawBitmap(bmp);
                        BRegion remaining = viewLocked->Bounds();
                        remaining.Exclude(&fSurface->fState.opaqueRgn.value());

@X547
Copy link
Owner

X547 commented Dec 19, 2024

Not here, but before if (mode == B_OP_ALPHA && fSurface->fState.opaqueRgn.has_value()) { line.

@kenz-gelsoft
Copy link
Contributor Author

OK, let's try with

~/src/haikuports/dev-libs/wayland-server/work-0.1.20241217/sources/wayland-server-c7f42756cd10868fc9ca24e6a34bf2c05472d37c> git diff
diff --git a/HaikuCompositor.cpp b/HaikuCompositor.cpp
index 2c1daf3..1756835 100644
--- a/HaikuCompositor.cpp
+++ b/HaikuCompositor.cpp
@@ -220,6 +220,7 @@ void WaylandView::Draw(BRect dirty)
                                mode = B_OP_COPY;
                                break;
                }
+               viewLocked->ConstrainClippingRegion(NULL);
                if (mode == B_OP_ALPHA && fSurface->fState.opaqueRgn.has_value()) {
                        viewLocked->ConstrainClippingRegion(&fSurface->fState.opaqueRgn.value());
                        viewLocked->SetDrawingMode(B_OP_COPY);

@kenz-gelsoft
Copy link
Contributor Author

It didn't help too...

@kenz-gelsoft
Copy link
Contributor Author

Do we have option that revert this and change firefox surfaces to have colorspace without alpha?

@kenz-gelsoft
Copy link
Contributor Author

I thought it has something to do with drawing bitmap twice in single view lock, so I tried splitting view locking, but it didn't help.

I felt Haiku has some special code path for clipping exclusion of complex region asynchronously, and the offloaded task has not enough priority, then it can't have chance to be executed in high CPU usage.

~/src/haikuports/dev-libs/wayland-server/work-0.1.20241217/sources/wayland-server-c7f42756cd10868fc9ca24e6a34bf2c05472d37c> git diff
diff --git a/HaikuCompositor.cpp b/HaikuCompositor.cpp
index 2c1daf3..2593378 100644
--- a/HaikuCompositor.cpp
+++ b/HaikuCompositor.cpp
@@ -205,7 +205,6 @@ void WaylandView::Draw(BRect dirty)
 
        BBitmap *bmp = fSurface->Bitmap();
        if (bmp != NULL) {
-               auto viewLocked = AppKitPtrs::LockedPtr(this);
                drawing_mode mode;
                switch (bmp->ColorSpace()) {
                        case B_RGBA64:
@@ -221,15 +220,25 @@ void WaylandView::Draw(BRect dirty)
                                break;
                }
                if (mode == B_OP_ALPHA && fSurface->fState.opaqueRgn.has_value()) {
-                       viewLocked->ConstrainClippingRegion(&fSurface->fState.opaqueRgn.value());
-                       viewLocked->SetDrawingMode(B_OP_COPY);
+                       {
+                               auto viewLocked = AppKitPtrs::LockedPtr(this);
+                               viewLocked->ConstrainClippingRegion(&fSurface->fState.opaqueRgn.value());
+                               viewLocked->SetDrawingMode(B_OP_COPY);
+                               viewLocked->DrawBitmap(bmp);
+                       }
+                       {
+                               auto viewLocked = AppKitPtrs::LockedPtr(this);
+                               BRegion remaining = viewLocked->Bounds();
+                               remaining.Exclude(&fSurface->fState.opaqueRgn.value());
+                               viewLocked->ConstrainClippingRegion(&remaining);
+                               viewLocked->SetDrawingMode(mode);
+                               viewLocked->DrawBitmap(bmp);
+                       }
+               } else {
+                       auto viewLocked = AppKitPtrs::LockedPtr(this);
+                       viewLocked->SetDrawingMode(mode);
                        viewLocked->DrawBitmap(bmp);
-                       BRegion remaining = viewLocked->Bounds();
-                       remaining.Exclude(&fSurface->fState.opaqueRgn.value());
-                       viewLocked->ConstrainClippingRegion(&remaining);
                }
-               viewLocked->SetDrawingMode(mode);
-               viewLocked->DrawBitmap(bmp);
        }
 
        fSurface->CallFrameCallbacks();

@kenz-gelsoft
Copy link
Contributor Author

It seems related to BWindows transaction, but we (wayland-server) or related apps doesn't seem to call this

~/src/haiku> grep -R BeginViewTransaction --exclude-dir=generated/ --exclude-dir=.git
docs/user/interface/Window.dox: \fn void BWindow::BeginViewTransaction()
headers/os/interface/Window.h:                  void                            BeginViewTransaction();
src/apps/cortex/TransportView/TransportView.cpp:                w->BeginViewTransaction();
src/apps/cortex/TransportView/TransportView.cpp:                w->BeginViewTransaction();
src/apps/cortex/TransportView/TransportView.cpp:                window->BeginViewTransaction();
src/apps/cortex/ValControl/ValControl.cpp:      pWnd->BeginViewTransaction();
src/apps/cortex/ValControl/ValControl.cpp:              pWnd->BeginViewTransaction();
src/apps/cortex/ValControl/ValControl.cpp:              window->BeginViewTransaction();
src/apps/icon-o-matic/generic/property/view/PropertyListView.cpp:                       Window()->BeginViewTransaction();
src/apps/patchbay/PatchView.cpp:        Window()->BeginViewTransaction();
src/apps/patchbay/PatchView.cpp:        Window()->BeginViewTransaction();
src/apps/patchbay/PatchView.cpp:        Window()->BeginViewTransaction();
src/apps/patchbay/PatchView.cpp:        Window()->BeginViewTransaction();
src/apps/processcontroller/MemoryBarMenu.cpp:   Window()->BeginViewTransaction();
src/apps/processcontroller/TeamBarMenu.cpp:     Window()->BeginViewTransaction();
src/apps/processcontroller/TeamBarMenu.cpp:             gCurrentThreadBarMenu->Window()->BeginViewTransaction();
src/kits/interface/ColumnListView.cpp:  Window()->BeginViewTransaction();
src/kits/interface/ColumnListView.cpp:          Window()->BeginViewTransaction();
src/kits/interface/Window.cpp:BWindow::BeginViewTransaction()

~/src/haikuports/dev-libs/wayland-server/work-0.1.20241217/sources/wayland-server-c7f42756cd10868fc9ca24e6a34bf2c05472d37c> grep -R BeginViewTransaction
~/src/haikuports/dev-libs/wayland-server/work-0.1.20241217/sources/wayland-server-c7f42756cd10868fc9ca24e6a34bf2c05472d37c> 

@X547
Copy link
Owner

X547 commented Dec 21, 2024

Draw transaction is automatically started when system calls BView::Draw.

@kenz-gelsoft
Copy link
Contributor Author

Other cases BWindow enters in transactions are while UPDATE ing with app_server

void
BWindow::DispatchMessage(BMessage* message, BHandler* target)
{
	if (message == NULL)
		return;

	switch (message->what) {
// snip
		case _UPDATE_:
		{
//bigtime_t now = system_time();
//bigtime_t drawTime = 0;
			STRACE(("info:BWindow handling _UPDATE_.\n"));

			fLink->StartMessage(AS_BEGIN_UPDATE);
			fInTransaction = true;

or when BWindow is initialized with _InitData() with non-zero bitmapToken

void
BWindow::_InitData(BRect frame, const char* title, window_look look,
	window_feel feel, uint32 flags,	uint32 workspace, int32 bitmapToken)
{
// snip
	fFlags = flags | B_ASYNCHRONOUS_CONTROLS;

	fInTransaction = bitmapToken >= 0;

@kenz-gelsoft
Copy link
Contributor Author

Draw transaction is automatically started when system calls BView::Draw.

I saw BView::ConstrainClippingRegion() calls _FlushIfNotInTransaction() and guessed it seems relating to this.

But you should have some expertise on app_server interaction, it just noise for you, sorry for disturbing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gnome Web's Settings window doesn't redraw anymore (regression)
2 participants