-
Notifications
You must be signed in to change notification settings - Fork 44
fix: Subsurface viewer - wells layer separate log layers #2674
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: master
Are you sure you want to change the base?
fix: Subsurface viewer - wells layer separate log layers #2674
Conversation
* Separated log curve visualization and selection to a dedicated composite layer * Moved various local utils into utility files
hkfb
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.
LGTM, only skimmed most of the moved code.
| if (typeof accessor !== "function") return accessor; | ||
|
|
||
| // @ts-expect-error -- Out is always a function here | ||
| return accessor(data, objectInfo); |
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.
I guess if you switch these two conditions around, then you wouldn't need the @ts-expect-error assertion?
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 issue is that the way typescript narrows things, Out can theoretically be a function, so typeof accessor === "function" does not narrow to a callable. I saw that deck.gl tended to use the same approach (@ts-expect-error included) so I just copied it. I'll add a comment to make it more clear
Took a bit long to get back to this, sorry about that
typescript/packages/well-log-viewer/src/Storybook/examples/MapAndWellLogViewer.tsx
Outdated
Show resolved
Hide resolved
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.
Are the coord changes expected ?
From (-1.77, 0.99) to (430732, 6481484) ?
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.
I think this one is okay? (-1.77, 0.99) seems to be some coordinate while the camera is still at (0,0), which doesn't make sense for these map stories. The new coordinate is actually within the bounds of the map.
I assumed there was some bug I coincidentally fixed here, when moving something to shared utilities so I left it as is.
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.
Are the style changes expected ?
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.
Yes, the log curve layer is now rendered a little bit further down to avoid clipping with the trajectory, so it's rendered slightly differently
In preparation for the data model change for the wells-layer (#2661 ), I did some preemptive refactoring and cleanup in the wells-layer to make it easier to work with. Most notably, I separated the logic for visualizing Well-log curves along the track as a dedicated sub-layer. This also fixes assorted bugs I discovered with the log layer as I was working on it.
Changes
wellsLayerfile; many of the local utilities have been moved to the layer's utility files