search

Seneral / Node_Editor_Framework

A flexible and modular Node Editor Framework for creating node based displays and editors in Unity
https://nodeeditor.seneral.dev
MIT License
2.01k stars 414 forks source link

Knob aspect ratio is wrong for non-square knob graphics. #174

Closed xyzDave closed 5 years ago

xyzDave commented 5 years ago

I've been playing with the framework and swapped the knob graphics. The non-square graphics I used were not rendered with the correct aspect ratio which revealed two possible bugs:

  1. in ConnectionKnob.cs private float knobAspect { get { return knobTexture != null? knobTexture.width/ knobTexture.height : 1; } } The line should be changed to: private float knobAspect { get { return knobTexture != null? (float) knobTexture.width/ (float) knobTexture.height : 1; } } The knobTexture.width and height need to be case to a float or the result otherwise the result is the result of dividing the two as integers (i.e. 48/30 would produce 1 not 1.6)

  2. in ConnectionKnob.cs In GetCanvasSpaceKnob, the calculation appears to be wrong. Currently it is: Vector2 knobSize = new Vector2 (NodeEditorGUI.knobSize * knobAspect,
    NodeEditorGUI.knobSize / knobAspect );     This should be changed to: Vector2 knobSize = new Vector2 (NodeEditorGUI.knobSize * knobAspect,
    NodeEditorGUI.knobSize );     (The "/knobAspect" removed) The aspect ratio value calculated here is used to multiply the horizontal to scale it up to keep the ratio correct for the vertical (to make sure the vertical is no bigger than "knobSize"), or divide the vertical by the aspect ratio to make sure the horizontal is no bigger than "knobSize", there is no need to do both.

I've tried it and it appears correct, so thought I'd add it as an issue. Cheers, xyzDave

Seneral commented 5 years ago

Sorry for the super late reply. Have included both fixes.