Spirograph (BeOS app) doesn't draw its background transparent

[jscipione]

Draws spirographs similar to a popular toy. Problem as stated by packager:

"But the transparency does not work on Haiku, so you will only see the top layer."

I've included some screenshots

[jscipione]

Screenshot on BeOS

[jscipione]

Screenshot on Haiku, notice the color #777477 which is B_TRANSPARENT_COLOR's hex value

[pulkomandy]

Peeking at the window/view structure with hey may provide some hints: how the views are laid out, which drawing modes they are using, etc.

[jscipione]

This appears to be the malfunctioning bit of code

int32 DrawView::Rewrite_image(DrawView*  data)
{
	rgb_color Transparent;
	memcpy (&Transparent, &B_TRANSPARENT_MAGIC_RGBA32, 4);

		// We'll add a drawing view to the bitmap
	int i;		// Index for the loop - we'll need it later
	BView* drawer = new BView (
		data->image->Bounds(),
		"drawer",
		B_FOLLOW_LEFT | B_FOLLOW_TOP,
		B_WILL_DRAW | B_SUBPIXEL_PRECISE);
			// If allocated successfully, add it to the bitmap
	if (drawer != NULL) {
		data->image->AddChild(drawer);
	}

			// Now, adding the image layers, one by one
			// The drawing mode must be set to B_OP_MODE, for the LowColor
			// of the drawing shall be treated as transparent.
			//
			// I must acquire the lock!
	if ( drawer->LockLooper() ) {
		drawer->SetLowColor(Transparent);
		drawer->SetViewColor(Transparent);
		drawer->SetDrawingMode(B_OP_OVER);
		for (i=0; i<MAX_LAYERS-1; i++) {
				// If current layer is not empty
			if ( (data->layers[i] != NULL) && ( (int )data->layers[i] != 0x19) ) {
					// Adding it to the bitmap, in B_OP_OVER mode
				drawer->DrawBitmap (data->layers[i]);
				drawer->Sync();
			}
		}
		drawer->UnlockLooper();
	}
	else {			// If I can't lock the BView, I alert the user.
		BAlert* cantLockLooper = new BAlert(NULL, "Cannot acquire the lock on the drawing element!", "Ok");
		cantLockLooper->Go();
	}

	// Now we may successfully delete the "drawer" BView
	data->image->RemoveChild (drawer);
	delete drawer;
	drawer = NULL;

	exit_thread(0);
	return 0;
}

[diver]

Was it fixed in hrev53713?

[X512]

Replying to diver:

Was it fixed in hrev53713?

In version 1.5 built from source issue is still present.

[pulkomandy]

I fixed it by changing the initialization of the bitmap in spirograph: at line 1215, SetBits is called with B_RGB32 colorspace, so the alpha channel is masked. There are comments in our SetBits implementation saying that B_RGB32 would import only 24 bits from the data, that doesn't seem to be exactly correct then?

It doesn't look perfect still due to the antialiasing being performed with the wrong color and the drawing not being done with alpha channel, but that's expected with the way the app works (BeOS had no antialiasing).

We can try to fix SetBits, it's already BeOS specific and ImportBits is used by Haiku apps because it makes more sense.

[jscipione]

Proposed fix: ​https://review.haiku-os.org/c/haiku/+/4319

[waddlesplash]

Should we place SetBits in "private" to prevent Haiku apps from using it, then?

[waddlesplash]

Merged in hrev55351.

[waddlesplash]

BTW, where is the source for Spirograph? I don't see it on GitHub in any of the usual places,

[jscipione]

SetBits() must be public for backwards compatibility with BeOS. We recommend against using in Haiku apps steering developers to ImportBits() instead. Note that this change to SetBits() is meant for increased BeOS app compatibility. Also note that the colorSpace == B_RGB32 condition has been in source since the beginning of SetBits() so I don't know the history/reasoning behind including it in the first place.

Spirograph 1.5 source is attached to this ticket.

​https://www.haiku-os.org/legacy-docs/bebook/BBitmap.html#BBitmap_SetBits ​https://www.haiku-os.org/docs/api/classBBitmap.html#a33e76109224b86faa983cbe9c2f17cd0

[pulkomandy]

From the be book:

If the color space mode is B_RGB32, the data should be triplets of three 8-bit components—red, green, and blue, in that order—without an alpha component. Although stored as 32-bit quantities with the components in BGRA order, the input data is only 24 bits in RGB order. Rows of source data do not need to be aligned.

​https://www.haiku-os.org/legacy-docs/bebook/BBitmap.html

If we follow the documentation, your change is incorrect as it removes the code that does exactly this. And this is the reason we introduced ImportBits in the first place: ImportBits has an API that makes sense, instead of this.

[jscipione]

Apparently the BeBook docs are wrong because Spirograph 1.5 doesn't draw the transparent background using our Haiku SetBits() while it did with calling SetBits() on BeOS R5. Unless I'm misunderstanding something, we have to follow what BeOS R5 actually did here, not what it was supposed to do. We should probably update our SetBits() documentation...

[waddlesplash]

private/public are source-only distinctions. Making SetBits private breaks source but not binary compatibility, which may be precisely what we want?

[jscipione]

Binary compatibility is what we're after, if we want to intentionally break source compatibility I suppose that would be ok.

[waddlesplash]

jscipione: We reverted this fix in hrev58322 due to #19246. Is some other fix for Spirograph still needed?

[waddlesplash]

In the meantime, I've created a repository for Spirograph: ​https://github.com/HaikuArchives/Spirograph

[korli]

I had to amend the behavior a bit: ​https://review.haiku-os.org/c/haiku/+/8562

[korli]

Applied in hrev58345.

[jscipione]

Thank you this makes sense now.