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

Implement ctrl+MMB drag zooming #71

Merged
merged 2 commits into from
Oct 11, 2021
Merged

Implement ctrl+MMB drag zooming #71

merged 2 commits into from
Oct 11, 2021

Conversation

DeMastah
Copy link
Contributor

  • Remove (unwanted?) shift of InfiniteCanvas in main scene
  • Fix 'enable_intput' and 'disable_intput" function name typos
  • Improve contrast between text and background on color button

* Remove (unwanted?) shift of InfiniteCanvas in main scene
* Fix 'enable_intput' and 'disable_intput" function name typos
* Improve contrast between text and background on color button
@mbrlabs
Copy link
Owner

mbrlabs commented Oct 10, 2021

Thanks for the PR! I'm going to check it out soon.

Copy link
Owner

@mbrlabs mbrlabs left a comment

Choose a reason for hiding this comment

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

This works and feels great! Code looks really good, just left some comments.

_mouse_wheel_counter = MAX_MOUSE_WHEEL_LEVEL
_current_zoom_level = 1.0 + _mouse_wheel_counter * _calc_zoom_increment()
func _do_zoom_scroll(step: int) -> void:
# NOTE: This *should* make zooming using scroll after having dragged "snap"
Copy link
Owner

Choose a reason for hiding this comment

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

That's normal. You can check in _input if an event is pressed (in this case that would mean the start of the scroll i guess) or not pressed (end of scroll). That way _do_zoom_scroll will only get called once.

Like so:

if event.pressed:
   _do_zoom_scroll(1)


export var zoom_curve: Curve
var _is_input_enabled := true
Copy link
Owner

Choose a reason for hiding this comment

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

Since we don't use the curve anymore i guess you can delete the zoom_curve.tres file completly.

@@ -74,7 +74,8 @@ func enable_tool(tool_type: int) -> void:
# -------------------------------------------------------------------------------------------------
func set_brush_color(color: Color) -> void:
_color_button.get("custom_styles/normal").bg_color = color
var text_color := color.inverted()
var lum := color.r * 0.2126 + color.g * 0.7152 + color.b * 0.0722
Copy link
Owner

Choose a reason for hiding this comment

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

That's great!

var zoom_center = anchor - offset
var ratio = 1.0 - _current_zoom_level/old_zoom
var ratio = 1.0 - target_zoom/_current_zoom_level
Copy link
Owner

Choose a reason for hiding this comment

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

Please add spaces between the operands and division sign

return zoom_curve.interpolate(progress)
func _to_nearest_zoom_step(zoom_level: float) -> float:
zoom_level = clamp(zoom_level, MIN_ZOOM_LEVEL, MAX_ZOOM_LEVEL)
zoom_level = round(log(zoom_level)/log(ZOOM_INCREMENT))
Copy link
Owner

Choose a reason for hiding this comment

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

Spaces

@DeMastah
Copy link
Contributor Author

Just pushed some changes. I realize that zooming using the scroll wheel feels slower (half as fast, I guess) now that it doesn't register twice. Fortunately, that can be remedied by simply increasing ZOOM_INCREMENT a tiny amount.

@mbrlabs
Copy link
Owner

mbrlabs commented Oct 11, 2021

Great work, thanks!

@mbrlabs mbrlabs merged commit bb1171c into mbrlabs:main Oct 11, 2021
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.

2 participants