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

ae.utils.graphics => renaming .w / .h to .width / .height? #17

Open
p0nce opened this issue Jun 22, 2015 · 5 comments
Open

ae.utils.graphics => renaming .w / .h to .width / .height? #17

p0nce opened this issue Jun 22, 2015 · 5 comments

Comments

@p0nce
Copy link

p0nce commented Jun 22, 2015

So I've been an ae.utils.graphics user and I don't have much complaints.
I know it is a breaking change and not very important, but .w and .h look a tiny bit terse to me.

@CyberShadow
Copy link
Owner

I'd like to do one more iteration of the graphics package with better range support (exposing the image as a range of rows, or range of pixels) so it's interoperable with std.algorithm and allows streaming image data, so I'll probably fold this change in there.

@p0nce
Copy link
Author

p0nce commented Nov 3, 2015

More suggestions:

  • use only one multiply in Image.scanline() instead of two: the common sub-expression isn't easy to merge, and I've found this function takes significant time in benchmarks. ImageRef doesn't have this problem. (trivial)
  • draw primitives with integer coordinates are not that useful. For a renderer I found that most often I want float coordinates and I rarely want non-AA versions. (lot of work)

@CyberShadow
Copy link
Owner

use only one multiply in Image.scanline() instead of two: the common sub-expression isn't easy to merge, and I've found this function takes significant time in benchmarks. ImageRef doesn't have this problem. (trivial)

Fixed

draw primitives with integer coordinates are not that useful. For a renderer I found that most often I want float coordinates and I rarely want non-AA versions. (lot of work)

Making the functions templated might Just Work in many cases. How about fixing this as needed?

@p0nce
Copy link
Author

p0nce commented Nov 3, 2015

Making the functions templated might Just Work in many cases. How about fixing this as needed?

You are right.

I've also noticed some naming/consistency problem in aaXXXX vs XXX nomenclature.
Please don't take it too seriously because I'll be super picky here, because in general I've been more than content with ae.utils.graphics. But it's so good one can't help to notice :)

  • softRing, softCircle and softRoundShape work with float coordinates but do not have the "aa" prefix. Which is probably because non-AA methods would be pretty undesirable indeed. I'd prefer "aa" names for those.
  • aaHline/aaVline/aaFillPoly don't exist (being pedantic)

In general I could live with the "aa" primitives having the default shorter name and the integer version having the longer names (reverse then current), or not even existing publicly. Overloading by coordinates format, could be nice but I fear for readability. Though it works well for CHECKED.

@CyberShadow
Copy link
Owner

No disagreements there. I'm actually not a fan of CHECKED though, because default parameters (template or not) don't stack. It's a holdover from ancient D1 code. Maybe it can be replaced with a Painter(bool checked) "namespace" (struct with static methods) that can be used like with (uncheckedPainter) { ... } OSLT, with aliases to the checked variant on the module level.

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

No branches or pull requests

2 participants