Normalize geometry arguments across all functions

[dankamongmen]

We take geometries in more than a dozen functions as parameters. We were handling them differently in different places: sometimes -1 meant "all available", sometimes 0 did (and negatives were all errors), for some any non-positive number was an error. All geometries and lenghts are now passed as unsigned, and 0 means "everything" where that is appropriate. Closes #1696.

[joseluis]

What about ncplane_dim_yx, which still expects int for the dimensions?

[dankamongmen]

What about ncplane_dim_yx, which still expects int for the dimensions?

pointers did not change with this PR. they might with #1777. i started down that road and the churn would be deafening. i'm completely unsure as to whether it's worthwhile.

[joseluis]

What about ncplane_dim_yx, which still expects int for the dimensions?

pointers did not change with this PR. they might with #1777. i started down that road and the churn would be deafening. i'm completely unsure as to whether it's worthwhile.

Ah ok. I'm a big fan of having as much type consistency in the library as possible. But yeah you'll have to decide how much churn is worthwhile...

[dankamongmen]

What about ncplane_dim_yx, which still expects int for the dimensions?

pointers did not change with this PR. they might with #1777. i started down that road and the churn would be deafening. i'm completely unsure as to whether it's worthwhile.

Ah ok. I'm a big fan of having as much type consistency in the library as possible. But yeah you'll have to decide how much churn is worthwhile...

it is truly immense, if i go all the way through and make them unsigned internally. if i held it at the API point, it would be less tremendous. perhaps that's a reasonable halfway point for 3.0.0.

[joseluis]

if i held it at the API point, it would be less tremendous. perhaps that's a reasonable halfway point for 3.0.0.

indeed, doing it incrementally sounds reasonable

[dankamongmen]

hrmmmmmmmmmmmm.

i'm suddenly wondering if we don't actually want to admit int rather than unsigned for position at all entry points, to indicate "use the current cursor location as the origin". that's what we do for e.g. ncplane_*_yx() and ncplane_erase_region().

hrmmmmmmmmmmmm.

ugh, yeah, this was a mistake. lengths as unsigned are fine, lower-right coordinate as unsigned are fine, but we really ought allow -1 to mean "current cursor position in this dimension". actually, hrmmm, maybe we want to allow that for bottom-right as well, so you can easily say e.g. "0,0 to current cursor."

yep, i fucked this up. oh well, glad it didn't get released. all coordinates are going to go back to int, and we're going to implement that semantic on all of them. @joseluis , hope this doesn't cause you to throw away too much work =[. but i think this is a useful generalization, and it brings these functions into line with others. if that's our goal (normalization), we need support this everywhere. oh well! man, my heart knew something was wrong with this work.

[joseluis]

try, fail, learn, try again :) it's the loop of success!

I'm thinking another option would be to interpret UINTMAX_MAX (which has the same bits representation as int -1) for "current cursor position", and has the advantage of enforcing the semantics of "no valid negative values", but I'm guessing maybe that's not as ergonomic or convenient for cultural reasons...

In Rust it's easier to express these semantics by using an Option enum wrapper, so that you can pass None to indicate no value or Some(val) to pass the magnitude, with val being the right type for the job, and everything is clear.

Anyhow the decision must be made according to what makes most sense for the C Api. If that's what your heart tells you then that's it, and I'll wrap around it no prob.

[dankamongmen]

I'm thinking another option would be to interpret UINTMAX_MAX (which has the same bits representation as int -1) for "current cursor position", and has the advantage of enforcing the semantics of "no valid negative values", but I'm guessing maybe that's not as ergonomic or convenient for cultural reasons...

indeed, only the USA, Liberia, and Myanmar still refuse to use metric types. what the rest of the world calls UINTMAX_MAX (btw i think you mean UINT_MAX there; UINTMAX_MAX is the maximum value of a uintmax_t) we call FOOTIELARGE; it's 5039 footies or about 2^50 - 1815. while you might think the 1815 a reference to el Rey Felón's triumphant entry to Valencia, i assure you no one in my country knows who Ferdinand VII is. statistically, i don't even know who he is. if asked, most would likely guess that he was the patron of christopher columbus, or perhaps the archduke of serbia or the person who assassinated the archduke of serbia, but either way they're pretty sure he controlled germany from its capital in seville. remember the maine!

i would worry about the ability for wrappers to access this preprocessor definition, and also given that the internals are gonna stay ints...

In Rust it's easier to express these semantics by using an Option enum wrapper, so that you can pass None to indicate no value or Some(val) to pass the magnitude, with val being the right type for the job, and everything is clear.

i wholeheartedly support you exposing it in this fashion.

Had I the heavens' embroidered cloths,
Enwrought with golden and silver light,
The blue and the dim and the dark cloths
Of night and light and the half light,
I would spread the cloths under your feet:
But I, being poor, have only my dreams;
I have spread my dreams under your feet;
Tread softly because you tread on my dreams.
--william butler yeats

Anyhow the decision must be made according to what makes most sense for the C Api. If that's what your heart tells you then that's it, and I'll wrap around it no prob.

tell you what i'm gonna do i'm get on my knees and put it in the Lord's hands. coincidentally, He's also colloquially known as FOOTIELARGE, caps and everything.

[joseluis]

Yeah I meant UINT_MAX...

I just found another case where FOOTIELARGE has to make a decision. I'm seeing all of the fields in ncvgeom are int but I suspect several of them could probably be uint, or are they using the negative side for something else?

[dankamongmen]

Yeah I meant UINT_MAX...

I just found another case where FOOTIELARGE has to make a decision. I'm seeing all of the fields in ncvgeom are int but I suspect several of them could probably be uint, or are they using the negative side for something else?

those can and probably ought all be unsigned

[joseluis]

those can and probably ought all be unsigned

that's great.

BTW do you know which of fields can have a valid 0 value? I'm guessing lenx & leny, are there any more?

[dankamongmen]

in ncvgeom?

if you pass a NULL nc, only pixy and pixx will be non-zero.
if you pass a NULL n, only maxpixel{yx}, cdim{yx}, and scale{yx} can be non-zero.
otherwise:

• leny/lenx and beg{yx} are whatever are supplied in vopts, 0 by default
• maxpixel{yx} are 0 if there are no maxima for bitmaps
• pix{yx}, cdim{yx}, rcell{yx}, scale{yx} are always non-0 in this case

[joseluis]

@dankamongmen then would it be safe to say that valid values of pix{yx}, cdim{yx}, rpix{yx}, rcell{yx} and scale{yx} are never gonna be 0?

[dankamongmen]

correct. you can get that for certain invocations of ncvisual_geom, though.

[joseluis]

correct.

ok that's great.

you can get that for certain invocations of ncvisual_geom, though.

yes, and that's why I'm creating a thin wrapper over ncvgeom for the rust bindings, that has every fields optional (except blitter which IIUC is the only one guaranteed to be always valid), and the fields contain something only when the underlying value is valid according to the rules you explained before... That ought to cut out some cognitive burden from the user...

[joseluis]

These are the new refactored methods:

• Nc.visual_geom(Option<&NcVisual>, Option<&NcVisualOptions>)
• NcVisual.geom(Option<&Nc>, Option<&NcVisualOptions>)

In case you want to check them.