Opened 8 years ago

Closed 3 years ago

Last modified 3 years ago

#12931 closed bug (fixed)

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

Reported by: jscipione Owned by: jscipione
Priority: normal Milestone: R1/beta4
Component: Kits/Interface Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description (last modified by 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

Attachments (3)

spirograph_med.jpeg (94.4 KB ) - added by jscipione 8 years ago.
Screenshot on BeOS
Spirograph.png (134.3 KB ) - added by jscipione 8 years ago.
Screenshot on Haiku, notice the color #777477 which is B_TRANSPARENT_COLOR's hex value
Spirograph-1.5-src.zip (25.6 KB ) - added by jscipione 8 years ago.

Download all attachments as: .zip

Change History (18)

by jscipione, 8 years ago

Attachment: spirograph_med.jpeg added

Screenshot on BeOS

by jscipione, 8 years ago

Attachment: Spirograph.png added

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

comment:1 by pulkomandy, 8 years ago

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.

by jscipione, 8 years ago

Attachment: Spirograph-1.5-src.zip added

comment:2 by jscipione, 8 years ago

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;
}

comment:3 by jscipione, 8 years ago

Description: modified (diff)

comment:4 by diver, 4 years ago

Was it fixed in hrev53713?

in reply to:  4 comment:5 by X512, 4 years ago

Replying to diver:

Was it fixed in hrev53713?

In version 1.5 built from source issue is still present.

comment:6 by pulkomandy, 4 years ago

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.

comment:8 by waddlesplash, 3 years ago

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

comment:9 by waddlesplash, 3 years ago

Milestone: R1R1/beta4
Resolution: fixed
Status: newclosed

Merged in hrev55351.

comment:10 by waddlesplash, 3 years ago

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

comment:11 by jscipione, 3 years ago

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

comment:12 by pulkomandy, 3 years ago

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.

comment:13 by jscipione, 3 years ago

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...

comment:14 by waddlesplash, 3 years ago

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

comment:15 by jscipione, 3 years ago

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

Note: See TracTickets for help on using tickets.