Skip to content

Conversation

@jepler
Copy link

@jepler jepler commented Jul 21, 2025

Over on Mastodon, Scott reported that NewRgn didn't work.

There are two problems: First, the API translator is treating RgnHandle as an opaque type (which it's not actually). Second, opaque type handling was wrong.

This fixes the 2nd problem, and also a problem with generated tables for describing aggregates of scalars (e.g., pointer-to-char) in non-opaque types.

image

@jepler
Copy link
Author

jepler commented Jul 21, 2025

Further report from mastodon:

However, this line is failing - it will cause the app to exit with error code 25:

qd.UnionRgn(window.visRgn, mBarRgn, window.visRgn)

If I change the target (third parameter) to another region, that seems to work okay:

foo = qd.NewRgn()
qd.UnionRgn(window.visRgn, mBarRgn, foo)

But if I then try to overwrite window.visRgn (which I don’t expect to work anyway, but just in case) then I get a Python exception, “TypeError: cannot assign to aggregate”.

foo = qd.NewRgn()
qd.UnionRgn(window.visRgn, mBarRgn, foo)
window.visRgn = foo

The original C code looks like this:

mBarRgn = NewRgn();
RectRgn( mBarRgn, &mBarRect );
UnionRgn( window->visRgn, mBarRgn, window->visRgn );
DisposeRgn( mBarRgn );

And here’s my Python version:

mBarRgn = qd.NewRgn()
qd.RectRgn(mBarRgn, mBarRect)
qd.UnionRgn(window.visRgn, mBarRgn, window.visRgn)
qd.DisposeRgn(mBarRgn)

@jepler
Copy link
Author

jepler commented Jul 28, 2025

Just an update on this: I have been working on this, but without conclusion so far. I need to get some clarity on how the different pointer and reference behaviors work before I can properly fix things; and I need to clean up the mkapi.py script so that I don't feel it's working against me.

Take the case of OffsetRect. In C, the prototype is OffsetRect(Rect *r, INTEGER dh, INTEGER dv). But in Python terms, since "everything's a reference", an argument produced by the Rect() constructor with a repr of <Rect STRUCT address> is appropriate, not a hypothetical RectPtr.

Things become muddy once you have a type (Region), a pointer (RegionPtr) and a handle (RgnHandle). If the C prototype is DisposeRgn(RgnHandle rh), it seems weird that the Python type would be a <RegionPtr PTR address> object .. but kinda it turns out that this is the case. It seems like the current version says it's working on <RgnHandle>s but it's somehow losing a level of indirection; anyway, it's a mess, and probably splatting system memory in a real bad way.

In the short term I have (non-pushed) work that is aimed at making this type handling stuff testable via the unix coverage build, instead of requiring to run in an emulator, by having a do-nothing implementation of some routines like NewRegion that can be built into a module and tested with micropython's regular testing system.

@jepler jepler force-pushed the region-fixes branch 2 times, most recently from 8967de9 to 4d29233 Compare July 31, 2025 20:15
@jepler
Copy link
Author

jepler commented Jul 31, 2025

I've updated this PR. Now some stuff about mkapi-generated bindings is tested, especially when it comes to RegionHandle/RegionPtr/Region & WindowPtr / GrafPtr / GrafPort. Now, the simulation looks slightly sane.

Without a complete test program, I wasn't able to confirm or refute the problems with UnionRgn.

@jepler jepler force-pushed the region-fixes branch 4 times, most recently from 99b67de to 56263ea Compare July 31, 2025 20:50
& do some testing of mkapi-generated bindings
@jepler
Copy link
Author

jepler commented Jul 31, 2025

qd.qdGlobals().thePort doesn't seem to refer to the right thing. The field defintion is:

static const mp_rom_obj_tuple_t common_103 = ROM_TUPLE(MP_ROM_INT(offsetof(QDGlobals, thePort) | UCTYPE_AGG(PTR)), MP_ROM_PTR((void*)&GrafPtr_obj));
…
{ MP_ROM_QSTR(MP_QSTR_thePort), MP_ROM_PTR((void*)&common_103) },

however, since the field is a UCTYPE_AGG(PTR) the 3rd arg should be the dereferenced type (GrafPort) not the pointer type (GrafPtr).

@jepler
Copy link
Author

jepler commented Jul 31, 2025

qdGlobals also seem to be behaving now:

>>> gbl = qd.qdGlobals()
>>> gbl.thePort
<struct PTR 38b5f2>
>>> gbl.thePort[0]
<GrafPort STRUCT 29f876
    device=0
    portBits=<BitMap STRUCT 29f878>
    portRect=<Rect STRUCT 29f886>
    visRgn=<struct PTR 29f88e>
...
>>> gbl.thePort[0].visRgn[0][0].rgnBBox
<Rect STRUCT 2a1af8
    top=0
    left=0
    bottom=292
    right=502>

The seemingly extraneous [0] are because Python doesn't have a dereference operator, so indexing is the only syntax that is available. A pointer must always be indexed at 0, while an array can be indexed at whatever number. (there's currently no bounds checking, ow.)

@jepler
Copy link
Author

jepler commented Jul 31, 2025

would () to dereference a pointer be any better? gbl.thePort().visRgn()().rgnBBox

or the special attribute named _? gbl.thePort._._.rgnBBox. yuck

Otherwise, Python doesn't have any kind of postfix operators to repurpose.

@jepler jepler merged commit b045a72 into m68k-micropython:main Aug 1, 2025
6 checks passed
@jepler jepler deleted the region-fixes branch August 1, 2025 14:41
@jepler
Copy link
Author

jepler commented Aug 1, 2025

I've added some fresh issues for things that are still rough edges. I think this is enough of an improvement to merit being merged.

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

Successfully merging this pull request may close these issues.

1 participant