-
Notifications
You must be signed in to change notification settings - Fork 2
Relative points #108
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
base: main
Are you sure you want to change the base?
Relative points #108
Conversation
88b4f5a to
b317eaa
Compare
5da6a76 to
b3d73ee
Compare
src/data/GraphPoints.ts
Outdated
| import {vec2, vec3} from 'gl-matrix'; | ||
| import {DataTexture} from '../renderer/DataTexture'; | ||
|
|
||
| export enum ClassModes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HierarchyType instead of ClassModes
b3d73ee to
6084097
Compare
6084097 to
3c0339a
Compare
cfa1721 to
2bf10a4
Compare
src/data/shaders/GraphPoints.fs.glsl
Outdated
| fragColor = texelFetch(uPointTexture, coords, 0); | ||
|
|
||
| uint i = 0u; | ||
| int classIndex = texelFetch(uParentTexture, coords, 0).x; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename classIndex to parentIndex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
| #define MODE_NONE 0u | ||
| #define MODE_ADD 1u |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename this file to hierarchyType.glsl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
Nithos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has an example been also added to the demos to see how this works?
| | Property | Type | Description | | ||
| | :--- | :--- | :--- | | ||
| | id | string - *optional* | Name of the point. Will be used as an ID when referencing point in node, edge, and label data. Will default to its index in the PointData array if left out. | | ||
| | parentId | string - *optional* | ID of the point which this point is the child of. Used to enable relative positioning of points and relative radius. Will default to *No Parent* if left out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ID of the parent point that this point ...
Does it have to be a direct parent, or can it be any ancestor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parentId put in here is the direct parent
| | :--- | :--- | :--- | | ||
| | positionHierarchyType | HierarchyTypes | Changes how point hierarchies changes the point positions. See [HierarchyTypes](./hierarchy-types.md) for more information. | | ||
| | radiusHierarchyType | HierarchyTypes | Changes how point hierarchies changes the point radius. See [HierarchyTypes](./hierarchy-types.md) for more information. | | ||
| | maxHierarchyDepth | number | Sets the maximum hierarchy depth that any point will have. Defaults to 100. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is the depth used? why is it important? when would I want to change this to be smaller / bigger?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the maxHierarchyDepth is basically only there to save the user from circular references in the data and stuff like that, which would otherwise crash the os (infinite loop in fragment shader is really not good). this should be increased if you expect the hierarchy depth in the data to be larger than the default value.
|
|
||
| export interface PointData { | ||
| id?: number | string; | ||
| parentId?: number | string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the readme you state this as a string why and should it also be a number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh i dunno which is the correct type :(
|
|
||
| public getPointByIndex(index: number): [number, number, number, number] { | ||
| public getPointByIndex(index: number, isRelative = false): [number, number, number, number] { | ||
| if(isRelative) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this correct? should the old logic be run when it is relative? seems reversed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in my opinion no, because the function (the way it worked before) makes the assumption that the points returned are relative to the world space origin. this is incredibly important for stuff like tooltips etc. if we were to break this assumption we will be breaking a lot of stuff.
a small example was added |
|
eta wen? |
No description provided.