Skip to content
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

AGS 4.0: expand DrawingSurface API with more shapes #2623

Open
ivan-mogilko opened this issue Dec 19, 2024 · 8 comments
Open

AGS 4.0: expand DrawingSurface API with more shapes #2623

ivan-mogilko opened this issue Dec 19, 2024 · 8 comments
Labels
ags 4 related to the ags4 development context: graphics context: script api type: enhancement a suggestion or necessity to have something improved

Comments

@ivan-mogilko
Copy link
Contributor

There's something about DrawingSurface that always bugged me, it's that most of its "Draw" functions are in practice "Fill" functions: they draw and fill a shape. The underlying library that does the drawing has separate functions for the most shapes, but the non-fill ones are not exposed in script API.

Whenever I needed to draw a rectangle, but not fill rectangle, in AGS, I have to write my own function that does 4 DrawLine calls. It's not hard to do, but feels strange that a more primitive function is not a part of the API while a fill function is.

A secondary thing, there are shapes which are supported by the allegro drawing lib (or rather were, since we stripped all the unused parts; so only in theory now), but were never exposed to API: ellipse, arc, polygon and spline. Some of these may be useful, and not easy to script yourself (except maybe a polygon).

So my proposal is to consider expanding DrawingSurface with 2 major additions:

  1. Support non-fill variants of shapes: Rectangle, Triangle, Circle.
  2. Support drawing more types of primitives. I might suggest:
    • Arc (a semi-circular line);
    • Pie (an arc with 2 radiuses), a frame or filled;
    • Ellipse, a frame or filled;
    • Polygon (passing a dynamic array of points), a frame or filled.

If we take the non-fill variants, there's a immediate problem: the DrawX names are already taken by the fill variants.
I'm not complete sure how to deal with this, but one of the options is to have a separate FillColor property. Actually, this may be a right thing to do in any case (maybe?).
Suppose we have DrawingColor and FillColor, and FillColor is COLOR_TRANSPARENT by default, then user may set either only DrawingColor or both, and that will affect a result of primitives drawing functions.
But this has to be thought over.

@ivan-mogilko ivan-mogilko added type: enhancement a suggestion or necessity to have something improved context: graphics ags 4 related to the ags4 development context: script api labels Dec 19, 2024
@ivan-mogilko ivan-mogilko changed the title AGS 4.0: consider expanding DrawingSurface API with primitives drawing AGS 4.0: expand DrawingSurface API with more shapes Dec 19, 2024
@AlanDrake
Copy link
Contributor

StrokeColor and FillColor, then perhaps assigning to DrawingColor overwrites both at once?

@ericoporto
Copy link
Member

ericoporto commented Dec 19, 2024

Any reason why the color isn't just a parameter in the function call itself? Otherwise every change of color requires an extra API call - so things are slower since every API call is slow.

https://love2d.org/wiki/love.graphics

In love they have a "enum" using strings that has a "mode" which can be either fill or not: https://love2d.org/wiki/love.graphics.ellipse

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Dec 19, 2024

Any reason why the color isn't just a parameter in the function call itself? Otherwise every change of color requires an extra API call - so things are slower since every API call is slow.

When you say that "every API call is slow" it sounds like there's something essentially wrong with the engine API. It is true that API call is relatively slower than a simple bytecode instruction, but that does not make it slow in absolute terms. The execution speed depends on multiple factors: what that engine function is doing, the context it's called in (is it called many thousands of times per game tick?), and so forth.

If there's a game script that requires switching drawing color so much that the game becomes slow, then likely drawing operations themselves will slow it down much more on their own. Maybe for such cases we'd need a different drawing API altogether.

In regards to your question though; I've also been thinking about passing color as a drawing functions parameter, and if it were just 1 color, then that would make sense. But seeing how we are already discussing having at least 2 colors above (like a "stroke" or "pen" color, and "fill" color), this makes me wonder if that would be convenient in the long term.

For example, judging by the love2d documentation that you linked, they also have separate functions that setup drawing colors, and other functions that setup things like line thickness etc.

@ericoporto
Copy link
Member

That is fine, the love2d API is interesting it also mixes things that are hardware accelerated, the only issue is it has no concept of surface. In any case, it solves the fill/line issue with an enum, which in our case maybe the enum would go to the end of the function call I think for compatibility?

About the API slowness, it's just that I observed in a lot of times the time of whatever is actually being done by the API call is irrelevant vs the API call itself, when using the VS profiler. So the more that can be done in a single call is usually preferred.

@ivan-mogilko
Copy link
Contributor Author

I suppose that SetPixel is a good candidate for passing color along, since it's unlikely to need any other settings, and this function might use some optimizing anyway. The question there is how to make it pass "use DrawingColor" by default; we might require a separate constant for that.

Other thing, if we have multiple colors, then we might have a function that sets them altogether.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Dec 19, 2024

About the API slowness, it's just that I observed in a lot of times the time of whatever is actually being done by the API call is irrelevant vs the API call itself, when using the VS profiler. So the more that can be done in a single call is usually preferred.

I honestly disagree that this should be prioritized when designing API. Indeed, the bytecode execution is slower than the language compiled to a machine code, and probably AGS interpreter is slower than others in particular. But there are matters of convenience and separating basic operations that are usually set as priorities for API.
If, on other hand, we'll keep thinking about this function call overhead and set less function calls as a priority, then the API may become very different from how it was designed until today. Alot of things may in theory be merged together in helper functions. A number of widely used algorithms that are usually done in script may in theory be moved to the engine as well, and be done faster there. But would that be convenient and justified?
EDIT: my meaning is, I would not want us to become paranoid over this.

I would instead suggest to find the practical cases when the use of engine API actually causes bad game performance, and invent separate, faster solutions for them. Like something that does a bulk operation, for example.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Dec 19, 2024

On another hand, if we managed to implement function overloading, then there will be more freedom in how to add new functions, as we may have the basic function and extended function variants with more parameters.
(See #2533)

@ivan-mogilko
Copy link
Contributor Author

I suppose that SetPixel is a good candidate for passing color along, since it's unlikely to need any other settings, and this function might use some optimizing anyway. The question there is how to make it pass "use DrawingColor" by default; we might require a separate constant for that.

Commenting on my own thought here; somehow i did not realize this, but that having any constants for "special color value" will be bad in the long run, as there's a pending suggesting of mixing alpha value into drawing color.

Which leaves us with the following choices (from what i see):

  • If (hypothetically) DrawingColor property is removed completely for the sake of passing color as parameter, then color would be an obligatory parameter;
  • If function overloading is supported, then we might have 2 variants, one that uses preset DrawingColor, and one that lets to pass one directly;
  • Before any of the above happens, the only way is to have a function with slightly different name which accepts "color" parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ags 4 related to the ags4 development context: graphics context: script api type: enhancement a suggestion or necessity to have something improved
Projects
None yet
Development

No branches or pull requests

3 participants