Add RFC for the attribute system - #4406
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Performance Benchmark Results
|
f0af96a to
00e40b4
Compare
Performance Benchmark Results
|
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Performance Benchmark Results
|
Performance Benchmark Results
|
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Performance Benchmark Results
|
Performance Benchmark Results
|
Performance Benchmark Results
|
| # Summary | ||
|
|
||
| Give every item flowing through the graph a set of named, typed attributes | ||
| next to its primary `element` value. Attributes are stored as a packed |
There was a problem hiding this comment.
I'm not sure it is necessary or desirable to have a primary element. For example you might wish to store a list containing x: f64 and y: f64 as attributes. It would not make sense to set one of these as the primary « element » ?
I think your approach would work just as well with the primary element removed. It would perhaps even simplify it somewhat.
There was a problem hiding this comment.
I agree with this somewhat, the asymmetry is definitely weird, I think mainly inherited from the "Old" Table type.
If we go all in on "dynamic" types I think we could get inspiration from NuShell's
structured data types.
There was a problem hiding this comment.
There are still some differences between the main type and the attributes (main type does not need to be Copy, can have a non trivial drop impl etc.) so there is at least some technical reason. The attributes are also ignored during the type analysis so they don't drive the monomorphization / implementation selection. On a ux side, I think the primary types would be needed to color the node graph wires right?
I'd suggest to use () as the carrier type if you want to basically have only attributes, but I don't have that strong of an opinion design wise
| A unit value component means the edge exists and carries only its | ||
| attributes (`_: ()` still means no edge at all). The name enters the | ||
| graph at a source node holding the constant text input, whose output | ||
| type is filled at graph compile time, where user-supplied names join |
There was a problem hiding this comment.
A user may currently choose to give a dynamic name (such as the current animation time as a string). Would this be forbidden under this proposal?
There was a problem hiding this comment.
Yes, this would require the use of a special dynamic attribute whisch is essentially a hashmap. We might hide the difference from the user, but this dynamic path would be a lot slower
| fn set_attribute<T, A, Y>( | ||
| _: impl Ctx, | ||
| element: T, | ||
| (_, attr): ((), Attr<Custom<A, Y>>), |
There was a problem hiding this comment.
What is A and Y? What is Custom?
There was a problem hiding this comment.
I this case A is an identifier to associte input with output, so bsically just some token. And Y would be the type (though this syntax needs some more work it's currently more of a functional draft, but not pretty)
| inner level from outside is an explicit map/enter construct, so "set on | ||
| the parent" and "map over the children" are visibly different graphs. |
There was a problem hiding this comment.
It is surely useful to allow mapping over children after constructing the list? For example, I might have a « Trace Raster » node which converts a raster image into several shapes. I would then like to give each shape its own opacity.
How could this be accomplished within this system?
| index and produces that copy's values. `level_extent = count` declares | ||
| the size of the new level, and the compiler derives the structural parts | ||
| from that one declaration: the multiplication with the carrier's extent, | ||
| and the index decomposition that routes an output lane to the right copy | ||
| and content item. The `emit(...)` tail marks the one-lane form and |
There was a problem hiding this comment.
I find this syntax quite hard to read and reason about. What is the motivation over just explicitly writing a for index in 0..count ?
There was a problem hiding this comment.
Because the loop is never materialized in the node itself, we instead poll the node 100 times each with a different index. This is basically the rust iterator approach, an explicit for loop would require an intermediate collect
There was a problem hiding this comment.
This all builds on the async execution / iterator design
| Switch takes two lazy inputs and returns one of them, for any carrier, | ||
| without an implementations list: |
| the default). A name that wants a distinguished unset declares an | ||
| `Option` value type on its marker. | ||
|
|
||
| Fields are `Copy`, and larger payloads go behind a pointer-sized field. |
There was a problem hiding this comment.
What would own the larger payloads? Is it necessary to clone them when cloning a table?
There was a problem hiding this comment.
The frame arena allocator so the allocations are cheap and you can share references to them as long as the graph is running
3 participants
No description provided.