Skip to content
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

Master Project - DepthMap & co. #3

Closed
wants to merge 360 commits into from
Closed

Conversation

Skgland
Copy link
Contributor

@Skgland Skgland commented Jun 26, 2021

Ports the changes by dwo and our own changes from Keith to klightd-vscode

Still Todo

  • remove tslint.json
  • probably remove packages/klighd-core/.gitignore
  • undo change to packages/klighd-core/LICENSE
  • check added SetSynthesesAction (probably remove)
  • check added SetSynthesesActionData (probably remove)
  • check added SetSynthesisAction (probably remove)
  • check added SetSynthesisRegistry (probably remove)
  • fix RangeOptions currently not working
  • client side only RenderOptions
  • update UpdateDepthmapModelAction/Command
  • expand/collaps is currently not working as expected
  • Basic Bookmark functionality
  • When RenderOptions with updateNeeded=false are changed we need to force a re-render
  • Add setting to allow user to disable animating GoToBookmarkActions

Drakae and others added 30 commits August 7, 2019 12:39
shadow of node is only shown if interactiveLayout is set
getLayers must be called before calling getLayerOfNodes
offset of the layers is already added in the getLayer method. If a node is moved above or below a layer only a layer constraint is set.
…ng to the diagram server in keith-diagram.
Region.absoluteBounds.width is always identically to Region.boundingRectangle.bounds.width and the height behaves respectively
@christoph-fricke christoph-fricke removed their request for review August 30, 2021 20:50
Copy link
Contributor

@christoph-fricke christoph-fricke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool to see the my ideas already applied to other work as well! Looks like they are some useful abstractions. 😄

I noticed a few things that might be improved. The one suggestion that absolutely must be applied is the relative import across package boundaries.

applications/klighd-cli/src/helpers.ts Outdated Show resolved Hide resolved
applications/klighd-cli/src/main.ts Outdated Show resolved Hide resolved
packages/klighd-core/src/bookmarks/bookmark-registry.ts Outdated Show resolved Hide resolved
Copy link
Member

@NiklasRentzCAU NiklasRentzCAU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In its current state this looks good to me. @christoph-fricke I would say we wait with merging this though so you and the project can have their end presentations with their own code base. We will merge this after you all are done with the semester.
Note: merging this will also require switching from a Language Server based on the latest KLighD release to a LS based on the current KLighD master due to a change in child area placement.

@NiklasRentzCAU NiklasRentzCAU mentioned this pull request Oct 11, 2021
NiklasRentzCAU added a commit that referenced this pull request Oct 11, 2021
The masters projects of WS20/21 and SS21 "Google Maps for Models", implementing browsing, scalability, and interactivity features and fixes to KLighD diagrams.

Squashed together into a single commit, as simultaneous changes to the repository and project structure makes a clean application of all commits to this repository not cleanly handleable by git.

The individual commits, as listed below, are accessible via PR #3 

* Add hierarchical expand/collapse to nodes, node visibility, title texts with scaling, line width scaling, small text svg simplification

As part of the master project Google Maps for Models, nodes have an expansion state, that can be set to expand/collapses according to zoom level and diagram dimensions, nodes that are not visible in the viewport can be collapsed, for collapsed nodes the title of the region can be scaled for readability as well as macro state labels, if there is just one in a node, line widths can be scaled to appear with constant width in diagram and small texts can be simplified with rectangles. This functionality can be configured in the Render Options of the diagram.

* Fix dependency between option classes, when to generate depthMap

* Add detection for super state titles, small improvement to expandCollapse

* Use old option scheme, increase absolute position error margins

* Use Map for RenderingOptions, Add comments, Fix Style

* Revert Range bound name change, Fix text simplification for title candidates and y position, Fix title scaling

* Remove gAttrs replication, Add original indentation for titles, Fix comments

* Add Javadoc comments, Some minor fixes

* Check only critical regions for expand/collapse, Minor fixes

* Make sure the next region is in a child area

* only run expandCollapse when viewport changes

* fix deprecation warnings

* remove unused functions

* simplify getExpansionState

* only initialize render context in SKGraphView not in every child

* don't save renderOptions in depthMap

and process all regions of the first non-empty depth

* don't save renderOptions in depthMap

and process all regions of the first non-empty depth

* fix checkCriticalRegions so that expandCollaps not does not need to be called recursive

* trim init and inithelper methods

* fix logic after merge

* simplify init further

* use getExpansionState to have a unified behavior

* boundingRectangle should always be present

as such the removed conditional would always evaluate to false

* when visible always expand regions without parents

* fix findRegionWithElement behaviour

* mark Region.absouluteBounds as optional and update comments

* reset DepthMap on Model change instead of creating a new instance

during the reset clear the parent/child/element associations of all old regions, so that the gc has an easier job reclaiming their memory,
this might help with the oom errors

change Region.children to be an Array instead of a Set as we already uphold the set property by construction

* add comment regarding last commits code

* also re-calculate if the threshold changes

* undo unintended changes in view.tsx and fix missing reset of isCompleteRendering in DepthMap.reset

* change checkCriticalRegions to do less work when parent is collapsed

* remove Region lookup by KNode id, instead save a reference to the Region in the KNode

* fix using wrong region

* Revert "fix using wrong region"

This reverts commit 2d64a4ac312a3039afc7968aa34fb0dbb567d378.

* Revert "remove Region lookup by KNode id, instead save a reference to the Region in the KNode"

This reverts commit 4fef24fbba89701ac1e6af368e8e52d20fe27ca8.

* determine position of regions in depthmap before rendering

* child region absolute bounds should be relative to parent region absolute bounds not the parent nodes relative bounds

* don't translate the childarea as the offset is already applied to all children

* fix isVisible

* fix initially incorrect expansion state of children

* remove visibility buffer and fix bug for implicit regions

* remove no longer necessary out commented code

* change titleMap from a Map to a Set as it is only ever used as a Set

* remove never read isCompleteReendering

* restructure so we do less work for the `$root` node

* make DepthMap in the SKGraphModelRenderer optional

* Instantiate Depthmap when new model arives

* Problem with the set Model Command right now the updateModel should work

* Added a module which updates the depthmap.
see keith-sprotty/src/update.
The Command is called from the keith-digram-server when ever the model got updated!

* only redraw whole graph when necessary

* Added a primitive Bookmark icon which works by storing a view and going back to it on clicking it again !

* Added Comments and removed one init method

* Removed View log and changed init to void

* reduce duplication in DepthMap.init

* Added Zoom Changed to render

* fix linter errors (at least some of them)

* fix more lint errors

* make Depthap "work"

* remove tslint.json

* remove unnecessary .gitignore file

* undo change to LICENSE

* remove some things accidentally added in merge

* remove SetSynthesesAction
* remove SetSynthesesActionData
* remove SetSynthesisAction
* remove SetSynthesisRegistry

* fix fallback when RenderOption is not found

* never use the DepthMap when the targetKind is hidden

* added Range Option for Expand collapse Threshhold

* Added Depthmap creation when setmodel / updateModel actions are done

* Fixed Depthmap only created when resize to fit is enabled

* False handeler for RangeOption fixed

* Added a simple bookmark pannel rightnow without any funktionality!

* implement updateNeeded for RenderOptions

to not refresh the diagram for client only options

* Added a bookmark icon in the general section

* Key Feature of adding Bookmarks work those Bookmarks dont have functionality right now

* created now Command for created Bookmarks right now only displays a log

* Bookmarks work properly now!
Added funktionality to GetBookmarkCommand

* fix fallback when RenderOption is not found

apparently I forgot to save some files last time I tried this

* adjust bookmark icon

* use a registry for bookmarks

* Update packages/klighd-core/src/bookmarks/bookmark-panel.tsx

apply suggestion

Co-authored-by: Christoph Fricke <[email protected]>

* fix extraneous }

* fix missing whitespace and draw title placeholder anyways

* fix missing/old license header on new files

* fix empty line and add missing param tags

* fix one to many whitespace

* work on review notes for bookmark-module.ts

* work on review notes for bookmark-panel.tsx

* work on review notes for bookmark-registry.ts

* use SetViewportAction for GoToBookmark

* rename Visibility enum and its variants

also update logic behind choosing detail level as it was not properly adjusted for three states from the original two

* work on review notes for depth-map.ts

* second attempt to fix copyright/license header

* remove lazy rendering again

* add a comment regarding nodes without a containing region

* fix access to field on undefined value in case parent is not a KNode

* fix typo and diverged initial value

* fix sliders behaving weird

* add launch config for chrome

* remove special caseing

* remove previously dead-code as isNodeTitle was never set

* add placeholder permanently and fix their positioning

* auto-format all files touched by the pr

* remove Region fields that are never written to or never read from

* remove unused DepthMap.titleMap

* really fix placeholder now, size increases correctly now

* remove unnecessary double if

* move Region definition and add Region references to KNode

* some clean up

remove never read regionMap
remove never used depth parameter from initHelper
correctly set containingRegion and providingRegion when it should be null

* more progress towards on demand region creation

* add tooltips for regions

* reorder initHelper and initKNode and remove execss parameters

* check scaling factor before dividing by it

* remove commented out code

* fix and simplify checkCriticalRegions

recursively setting OOB or updating detail level should be always necessary and not only when a parent is present with a different visibility.
As a result handling the children separately is not necessary

* prepare for creating Regions on demand during rendering

while rendering we need to compute and set the detail level when we create the Region as updateDetailLevels is only performed once at the start of rendering for existing Regions and will only be redone at the next rendering, but we require it to be also set for new Regions for them to be rendered

* again move Region definition and references

* when lazy initializing Regions we can't store them on the SKNode as this will not persist, so we move them back to a Map in the DepthMap
* as a Result we can move the definition of the Region class back to klighd-core

* remove never read Region.elements

* adapt tooltips to show parent name as well

* clear the map instead of creating a new one overwriting the old one

* force a client-side only render update on render option change

* rename field to better match its use

* fix disabling smartzoom not working correctly

* add a preference to disable animating bookmark goto

* start work on bookmark url

* make BookmarkUrl work

* rework how bookmarks are displayed

add a copy to clipboard button, nyi

* add a GoToBookmarkCommand so we don't need the elementId as part of the bookmark

internally delegate to SetViewportCommand after we retrieve the id from the model

* add copy/past and delete for Bookmarks

* add an index to bookmarks

this replaces the array index, so that they won't renumber when a bookmark is deleted

this is also not saved for copy/past so that loaded bookmarks get a new uniue id

* remove trivial cast

* restore super call as we no longer reuse children

* fix button title

* also add the initial bookmark to the list of bookmarks

* remove unnecessary parameter/properties from UpdateDepthmapModelAction

* move detail level back to depth-map and remove unused DetailReference

* Revert "restructure so we do less work for the `$root` node"

This reverts commit f419ef3.

We can't reorder this code as we depend on some side-effect of the involved calls

# Conflicts:
#	packages/klighd-core/src/views.tsx

* handle a missing KRenderingLibrary less disruptive

* remove refreshNeeded for RenderOption

* remove tooltips for regions completely

* Make region titles overlay the region until visible text is blocked

* Change threshold for background of titles to 4

* make the bookmark name be user definable

* fix pasting of bookmarks

* have edit button focus the text field

* change scaling and shape of zoom placeholder for regions

* Add title overlay threshold option as slider

* make title overlay background slightly transparent

* remove unused lambda parameter

* fix spelling

* consistently spell DepthMap with a capital M outside of file names

* more fixed spelling

* add smoother transition form overlay title to normal title

* Only draw placeholder for smart zoom related collapsed regions

* Fix smoothing of title overlay

* add comments for title overlay

* Fix misplaced and rotated titles

* handle float parsing failure

* make methods protected for easier extensibility

* move default values to a centralized place

* fix outdated comment

* change Region.absoluteBounds to Region.absolutePosition

Region.absoluteBounds.width is always identically to Region.boundingRectangle.bounds.width and the height behaves respectively

* fix incorrect cross package imports

* make every BookmarkRegistry change go through an action

* Add title overlay for titles, that are not region titles, too

* add type predicates for bookmark actions

* use type predicates to distinguish bookmark actions

* move viewport check and construction to core

* have CreateBookmarkCommand dispatch an AddBookmarkAction

also implement undo/redo

* move title and positions map for title overlay to the rendering context

Co-authored-by: Jette Petzold <[email protected]>
Co-authored-by: Connor Schoenberner <[email protected]>
Co-authored-by: Soeren Domroes <[email protected]>
Co-authored-by: David Wolff <[email protected]>
Co-authored-by: Bennet Bleßmann <[email protected]>
Co-authored-by: Mika Pöhls <[email protected]>
Co-authored-by: Felix Jöhnk <[email protected]>
Co-authored-by: Felix Jöhnk <[email protected]>
Co-authored-by: Bennet Bleßmann <[email protected]>
Co-authored-by: Christoph Fricke <[email protected]>
@Skgland
Copy link
Contributor Author

Skgland commented Oct 11, 2021

If I am not mistaken then this was squashed and merged in #19, as such closing this.

@Skgland Skgland closed this Oct 11, 2021
@Skgland
Copy link
Contributor Author

Skgland commented Oct 11, 2021

Reopening as #19 did not merge to main

@Skgland Skgland reopened this Oct 11, 2021
@NiklasRentzCAU
Copy link
Member

Now it is merged and therefore closed.

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.

8 participants