Unified layout and limits functions for vtile and htile.

[bruno-j-nicoletti]

Added functions to get values out of rect, point and view_stretch by axis.

Refactored the tile code so that the limit and layout method on vtile_element and htile_element call common templated code using that axis dependent code. Gives single code path for both.

[bruno-j-nicoletti]

Would an operator [] be useful here as well?
constexpr float operator[](bool is_x_axis) const

[djowel]

Looking at this a bit, I think it can be done non-intrusively of point and rect, via free functions.

Axis can be an enum class:

enum class axis { x, y };

Accessing axis specified coordinates can be done non intrusively:

constexpr float const& coord(point const& p, axis a) 
{
   return a==axis::x ? p.x : p.y;
}
      
constexpr float& coord(point& p, axis a) 
{
   return a==axis::x ? p.x : p.y;
}

Usage:

auto v = coord(p, axis::x); 

This is a lot better than

auto v = p.coord(true);

Same for rect

[bruno-j-nicoletti]

Looking at this a bit, I think it can be done non-intrusively of point and rect, via free functions, if you treat point and rect as fixed sized arrays of floats via operator[](std::size_t). The only change will be the addition of operator[]

That's how I normally write my vector/point classes, not that I do that any more, currently my code uses the Eigen library, https://gitlab.com/libeigen/eigen. The only issue is then is bounds checking, which a bool saves you from in the 2D case.

To me, logically treating bounding boxes/rects as flat arrays doesn't make as much sense as it does for vectors/points. It's two point values representing the min/max values on the axii in your N dimensional space.

Happy to flip it to free functions.

[bruno-j-nicoletti]

Ha! You edited your comment as I was posting. My other unstatate idea was for an axis enum to do the indexing into points/rects as opposed to a bool.

[djowel]

Ha! You edited your comment as I was posting. My other unstatate idea was for an axis enum to do the indexing into points/rects as opposed to a bool.

Yes, sorry. my mind was wandering as I explored some more.

[djowel]

Happy to flip it to free functions.

Yes. The general rule is ... If it can be a free function, make it so.

[djowel]

The only issue is then is bounds checking, which a bool saves you from in the 2D case.

True! And that is why I took it back :-)

[bruno-j-nicoletti]

Only issue then is the axis specifiying rect constructor.

constexpr rect::rect(axis a, float this_axis_min, float other_axis_min, float this_axis_max, float other_axis_max);

Given it's creating a rect I'd argue this shouldn't be a free function.

[djowel]

Only issue then is the axis specifiying rect constructor.

constexpr rect::rect(axis a, float this_axis_min, float other_axis_min, float this_axis_max, float other_axis_max);

Given it's creating a rect I'd argue this shouldn't be a free function.

constexpr  rect make_rect(axis a, float this_axis_min, float other_axis_min, float this_axis_max, float other_axis_max);

similarly:

constexpr  point make_point(axis a, float this_axis_min, float other_axis_max);

[bruno-j-nicoletti]

I'm removing my camelHump method names and refactoring it now.

Looking at the code I'd argue for an pair of point::operator[](axis) methods, I think the version with the [] operator is clearer code as it's a familiar paradigm for coding.

     // Free function
     coord(limits.min, AXIS) += coord(el.min, AXIS);

     // operator
     limits.min[AXIS] += el.min[AXIS];

You can't write [] operators as free functions.

[djowel]

Looking at the code I'd argue for an pair of point::operator[](axis) methods, I think the version with the [] operator is clearer code as it's a familiar paradigm for coding.

Go for it! I like it!

[djowel]

Looking at the code I'd argue for an pair of point::operator[](axis) methods, I think the version with the [] operator is clearer code as it's a familiar paradigm for coding.

Go for it! I like it!

Except for the ALLCAPS :-P Haha.

[djowel]

I'm removing my camelHump method names and refactoring it now.

WeDontNeedEmHumps :-)

[bruno-j-nicoletti]

snakes_are_evil!

[bruno-j-nicoletti]

Looking at the code I'd argue for an pair of point::operator[](axis) methods, I think the version with the [] operator is clearer code as it's a familiar paradigm for coding.

Go for it! I like it!

Except for the ALLCAPS :-P Haha.

It's a template arg, I'll flip it to your stylee.

[djowel]

snakes_are_evil!

HumpsAreForTemplateTypenames :-) We love evil_snakes everywhere else.

[djowel]

It's a template arg, I'll flip it to your stylee.

If it's a non-type template arg, then it's also an evil_snake :-)

[djowel]

snakes_are_evil!

HumpsAreForTemplateTypenames :-) We love evil_snakes everywhere else.

https://bit.ly/4bo0mID

[bruno-j-nicoletti]

snakes_are_evil!

HumpsAreForTemplateTypenames :-) We love evil_snakes everywhere else.

https://bit.ly/4bo0mID

If we are being pedantic, from that thread....

Never start an identifier with an underscore, and don't have a double underscore anywhere inside an identifier. Those are reserved for the implementation.

My member variuables end with an underscore as opposed to starting with them.

:-P

[bruno-j-nicoletti]

Done!

[djowel]

My member variuables end with an underscore as opposed to starting with them.

:-P

That is actually wrong. The standard says never start with TWO underscores. But also never start a underscore and a cap, like _BAD

[djowel]

My member variuables end with an underscore as opposed to starting with them.
:-P

That is actually wrong. The standard says never start with TWO underscores. But also never start a underscore and a cap, like _BAD

Basically...

The use of two underscores ( __ ') in identifiers is reserved for the compiler's internal use according to the ANSI-C standard. Underscores ( _ ') are often used in names of library functions (such as " _main " and " _exit "). In order to avoid collisions, do not begin an identifier with an underscore.

BUT!!! C++ is scoped. Take that recommendation as old-fashioned, pre namespaces and strict scopes.

[bruno-j-nicoletti]

My member variuables end with an underscore as opposed to starting with them.
:-P

That is actually wrong. The standard says never start with TWO underscores. But also never start a underscore and a cap, like _BAD

Basically...

The use of two underscores ( __ ') in identifiers is reserved for the compiler's internal use according to the ANSI-C standard. Underscores ( _ ') are often used in names of library functions (such as " _main " and " _exit "). In order to avoid collisions, do not begin an identifier with an underscore.

BUT!!! C++ is scoped. Take that recommendation as old-fashioned, pre namespaces and strict scopes.

I learn something new everyday.

[djowel]

My member variuables end with an underscore as opposed to starting with them.
:-P

That is actually wrong. The standard says never start with TWO underscores. But also never start a underscore and a cap, like _BAD

Basically...
The use of two underscores ( __ ') in identifiers is reserved for the compiler's internal use according to the ANSI-C standard. Underscores ( _ ') are often used in names of library functions (such as " _main " and " _exit "). In order to avoid collisions, do not begin an identifier with an underscore.
BUT!!! C++ is scoped. Take that recommendation as old-fashioned, pre namespaces and strict scopes.

I learn something new everyday.

Summary:

1. Reserved in any scope, including for use as macros:

• identifiers beginning with an underscore followed immediately by an uppercase letter
• identifiers containing adjacent underscores (or "double underscore")

2. Reserved in the global namespace:

• identifiers beginning with an underscore

[bruno-j-nicoletti]

Done

[djowel]

One thing though: we will have to the same with the skia_2024 branch. Then, we'll have to merge master with skia_2024 (master tracks skia_2024). I keep both merged and in sync with the objective that they will one day be united. Obviously, there will be conflicts all over the place. I can do the master -> skia_2024 merge if you can do the port to skia_2024.

[bruno-j-nicoletti]

I can't get onto this again for a few days. The skia_2024 modifications would mean applying the rect/point changes to the rect/point classes in the artist module wouldn't it and refactoring the tile classes appropriately?

[djowel]

I can't get onto this again for a few days. The skia_2024 modifications would mean applying the rect/point changes to the rect/point classes in the artist module wouldn't it and refactoring the tile classes appropriately?

I suspect all that is needed is to apply the rect/point changes. The tiles will use that automatically by a simple using artist::rect and using artist::point which should already be there.

[djowel]

In the meantime, I changed the target to a feature branch tile-refactoring, so I can continue with documentation which requires regular sync from skia_2024 and master.

[djowel]

OK to merge so far?

[bruno-j-nicoletti]

OK to merge so far?

I haven't fixed the formatting yet.

[bruno-j-nicoletti]

Just made the last set of changed you suggested. Feel free to merge.