[rmodels] Fix aspect ratio calculation in DrawBillboardPro

[jmrico01]

Hello! Not sure if I misunderstood the code/API, but I believe the aspect ratio correction in DrawBillboardPro is incorrect. I would expect it to calculate sizeRatio such that sizeRatio.x / sizeRatio.y == source.width / source.height, or in other words match the aspect ratio of source. The current code is definitely not doing that. I was writing some custom 3D text rendering using the billboard functions and the sizing was very confusing and unexpected.

Hehe, I see this was brought up in #2494 before but still seems off. I think the suggested fix was incorrect.

NOTE - I am also concerned people are relying on incorrect behavior currently and this change will affect them. I actually have some code that would be affected, and I can't think of an alternative. Basically, I would like to draw billboards that are a flat color, without any texture. What I was doing is using a texture that was a 1x1 white pixel, but if the aspect ratio is forced to match the texture, I won't be able to draw anything flatter than a square 😢 maybe a new API is in order? Will think about specific suggestions...

EDIT: tbh, I think there should just be a version of this API that doesn't modify size behind the scenes and will stretch the texture, if that's what you want.

[raysan5]

@jmrico01 Please, could you show some screenshots with the issue and the fix?

[bohonghuang]

If we scale the original sizeRatio by source.height, we get something like Vector2 sizeRatio = { size.x*source.width, size.y*source.height }. If DrawBillboardPro expects to maintain the aspect ratio of the billboard when the input size is a uniform scaling vector (such as (Vector2){ size, size } used in DrawBillboard), the original calculation method of sizeRatio is correct.

[jmrico01]

@jmrico01 Please, could you show some screenshots with the issue and the fix?

Sorry it took me a while, but here's an example of the issue with screenshots. What I am doing here is using the following square texture (16x16)

I am calling DrawBillboardPro with the same camera, texture, source rectangle (x=0, y=0, width=16, height=16), position, up, no rotation, and WHITE tint. I am passing in 2 different sizes and posting a screenshot of the result before and after my change.

Size 2 x 4

Before

After

Size 4 x 2

Before

After

As you can see, the current code doesn't do what it claims, which is to maintain the aspect ratio of the source rectangle (in this example that is 16 x 16, aspect ratio of 1). In both cases it drew a stretched texture. After my change, the aspect ratio is maintained while matching the requested Y size, so the 2 x 4 is actually rendered as 4 x 4, and the 4 x 2 is rendered as 2 x 2. Hope this makes sense.

If we scale the original sizeRatio by source.height, we get something like Vector2 sizeRatio = { size.x*source.width, size.y*source.height }. If DrawBillboardPro expects to maintain the aspect ratio of the billboard when the input size is a uniform scaling vector (such as (Vector2){ size, size } used in DrawBillboard), the original calculation method of sizeRatio is correct.

I'm not sure I follow your argument. I guess you're saying the original code is correct for the case where the input size is a uniform scaling vector, like NxN? But per my example above, it's not correct for other values of size.

[bohonghuang]

After spending some time playing with DrawBillboardPro, especially when I implemented a feature-rich particle effect system based on it, I still haven't found any issues with the original implementation in Raylib. Based on my understanding, you may expect that DrawBillboardPro should always maintain the same aspect ratio as the source. However, the size variable is used to control the scaling of the billboard in both dimensions with different ratios. If we were to follow your implementation approach and ignore size.x, DrawBillboardPro would only achieve uniform scaling, which wouldn't be considered Pro. In the scenario where the aspect ratio of a billboard should match its source, all you need is to ensure size.x == size.y.

[raysan5]

@bohonghuang I reviewed this issue and I agree with your comment.

Still, I'm wondering if DrawBillboardPro() should be more aligned with DrawTexturePro() and allow any destination rectangle size (destSize) provided by user, instead of forcing a size ratio internally:

    Vector2 sizeRatio = { size.x*fabsf((float)source.width/source.height), size.y };

What do you think?

[bohonghuang]

@bohonghuang I reviewed this issue and I agree with your comment.

Still, I'm wondering if DrawBillboardPro() should be more aligned with DrawTexturePro() and allow any destination rectangle size (destSize) provided by user, instead of forcing a size ratio internally:

    Vector2 sizeRatio = { size.x*fabsf((float)source.width/source.height), size.y };

What do you think?

@raysan5 That would make sense. The size currently represents the scale based on the assumption that the billboard's original size is { (float)source.width/source.height, 1.0f }, instead of the actual billboard size. However, to achieve consistency with DrawTexturePro, the definition of the origin parameter also needs to be considered. In DrawTexturePro, when this parameter is { 0.0f, 0.0f }, the origin is at the top-left corner of dest, whereas in DrawBillboardPro, the origin is at the center of the billboard. Additionally, in DrawTexturePro, the origin is not normalized, meaning it is related to the size (dest.width and dest.height), which is not the case in the current DrawBillboardPro.

[raysan5]

@bohonghuang Thanks, good points. If we want to keep full consistency with DrawTexturePro(), position, size and origin should be reviewed. Could you review it?

[jmrico01]

Thanks for the clarifications and suggestions! I agree with making this API more consistent with DrawTexturePro, and in hindsight I don't think my change is the correct one with the "Pro" API as @bohonghuang said. Happy for this PR to be closed if the above changes are implemented. I just think it's a bit confusing as is.

[bohonghuang]

@bohonghuang Thanks, good points. If we want to keep full consistency with DrawTexturePro(), position, size and origin should be reviewed. Could you review it?

@raysan5 Certainly. It might be easier to uncover the potential issues by migrating my application that heavily uses DrawBillboardPro.