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

Various changes #1

Closed
wants to merge 1 commit into from
Closed

Various changes #1

wants to merge 1 commit into from

Conversation

modulovalue
Copy link

Hello Steve,
Thank you for porting perfect freehand to dart.
I was curious what kind of dart and flutter code someone with programming experience, but new to Dart, would write.

You said on twitter that you are open to feedback, so here are some suggestions.

The most important changes in this PR are the following:

  • extracted dart only logic into its own package (FYI you could use dart instead of typescript for deploying a js version of perfect freehand, see: https://dart.dev/tools/dart2js)
  • replaced streamcontrollers with setState (they are not needed in this case, setState is enough to notify the UI to repaint)
  • made some models const-able (the linter is able to highlight constructor calls that can be made constant. This will be more efficient in Dart as such models will only have to be allocated once.)
  • removed the set invocation from some setState calls which look like closures but aren't closures (closures don't need an arrow (abc) {} / function expressions don't need curly braces (abc) => ...

Some of the more subjective changes are:

  • removed newlines for clarity
  • added trailing commas to help dartfmt
  • replaced ternary operators with ifelses for clarity
  • sorted methods in the main widget topologically which is more idiomatic in dart than a reverse topological sort.

FYI: Flutter supports reading out various information from pressure sensitive input devices such as the Apple Pen on iPads.

Thanks again :)

- extracted dart only logic into its own package 
- removed newlines for clarity
- added trailing commas to help dartfmt
- replaced ternary operator with ifelses for clarity
- replaced streamcontrollers with setState
- made some models able to be const
- sorted methods in the main widget topologically which is more idiomatic in dart than a reverse topological sort.
- removed set syntax from some setState calls which look like closures but aren't closures (closures don't need an arrow / function expressions don't need curly braces)
@modulovalue
Copy link
Author

modulovalue commented Oct 4, 2021

I'm not going to be able to merge his PR [...]

No problem!

@modulovalue modulovalue closed this Oct 4, 2021
@steveruizok
Copy link
Owner

But seriously, thanks! I've moved over most of your changes into the main branch. With the exception of removing line newlines—I think our eyes might work a bit differently. ;)

adil192 added a commit that referenced this pull request Feb 6, 2024
[ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: 'dart:ui/lerp.dart': Failed assertion: line 19 pos 10: '<optimized out>': Cannot interpolate between finite and non-finite values
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:51:61)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)
#2      lerpDouble (dart:ui/lerp.dart:19:10)
#3      PointVector.lerp (package:perfect_freehand/src/types/point_vector.dart:54:7)
#4      getStrokeOutlinePoints (package:perfect_freehand/src/get_stroke_outline_points.dart:238:31)
#5      getStroke (package:perfect_freehand/src/get_stroke.dart:19:10)
#6      Stroke._getPolygon (package:saber/components/canvas/_stroke.dart:200:21)
#7      Stroke._updatePolygon (package:saber/components/canvas/_stroke.dart:48:16)
#8      Stroke.polygon (package:saber/components/canvas/_stroke.dart:38:32)
#9      Stroke.toSvgPath (package:saber/components/canvas/_stroke.dart:247:9)
#10     EditorExporter.generatePdf.<anonymous closure>.<anonymous closure> (package:saber/data/editor/editor_exporter.dart:111:52)
#11     CustomPaint.paint (package:pdf/src/widgets/basic.dart:623:25)
#12     SingleChildWidget.paintChild (package:pdf/src/widgets/widget.dart:320:14)
#13     ConstrainedBox.paint (package:pdf/src/widgets/basic.dart:449:5)
#14     StatelessWidget.paint (package:pdf/src/widgets/widget.dart:260:15)
#15     Page.paint (package:pdf/src/widgets/page.dart:246:13)
#16     Page.postProcess (package:pdf/src/widgets/page.dart:179:5)
#17     Document.save (package:pdf/src/widgets/document.dart:130:14)
#18     EditorState.exportAsPdf (package:saber/pages/editor/editor.dart:1213:72)
<asynchronous suspension>
#19     _ExportBarState._onPressed.<anonymous closure>.<anonymous closure> (package:saber/components/toolbar/export_bar.dart:38:29)
<asynchronous suspension>
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.

None yet

2 participants