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

Add a synthesis hook to configure top-down layout for SCCharts #127

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Eddykasp
Copy link
Contributor

@Eddykasp Eddykasp commented Nov 8, 2024

This synthesis hook adds a very basic configuration of top-down layout for SCCharts.
depends on eclipse/elk#1089

  • check that the hook works for the currently released versions of KLighD and ELK

@Eddykasp Eddykasp added the enhancement New feature or request label Nov 8, 2024
@Eddykasp Eddykasp self-assigned this Nov 8, 2024
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.

If it also works with the last ELK/KLighD releases, I'd be happy to have it in the semantics master! Alex would need to review first before merging, though.

Comment on lines 42 to 56
public static final SynthesisOption USE_TOPDOWN_LAYOUT =
SynthesisOption.createCheckOption(TopdownLayoutHook, "Topdown Layout", false)
.setCategory(GeneralSynthesisOptions::LAYOUT)

public static final SynthesisOption TOPDOWN_HIERARCHICAL_NODE_WIDTH =
SynthesisOption.createRangeOption("Topdown Hierarchical Node Width", 50.0f, 5000.0f, 1.0f, 150.0f)
.setCategory(GeneralSynthesisOptions::LAYOUT)

public static final SynthesisOption TOPDOWN_HIERARCHICAL_NODE_ASPECT_RATIO =
SynthesisOption.createRangeOption("Topdown Hierarchical Node Aspect Ratio", 0.2f, 5.0f, 0.01f, 1.41f)
.setCategory(GeneralSynthesisOptions::LAYOUT)

public static final SynthesisOption SCALE_CAP =
SynthesisOption.createCheckOption(TopdownLayoutHook, "Enable Scale Cap", true)
.setCategory(GeneralSynthesisOptions::LAYOUT)
Copy link
Member

Choose a reason for hiding this comment

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

  1. Try to always use the create... method using the class, that causes better IDs for the options to be generated.
  2. please use the setDescription method for each new option: This creates a hover-feedback popup for all options better describing what the option does. For example, I don't really get just from the name what "Enable Scale Cap" in the layout category should do.

Copy link
Member

Choose a reason for hiding this comment

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

Please order the options the same way as they are used in the displayed synthesis options, or at least alphabetical

.setCategory(GeneralSynthesisOptions::LAYOUT)


// public static final SynthesisOption
Copy link
Member

Choose a reason for hiding this comment

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

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants