Reputation: 1551
This is the main.dart file of my app:
import 'package:flutter/material.dart';
main() {
runApp(
new MaterialApp(
title: 'Flutter Database Test',
theme: ThemeData(
textTheme: TextTheme(
display1: TextStyle(fontSize: 24.0, color: Colors.white),
display2: TextStyle(fontSize: 24.0, color: Colors.grey),
display3: TextStyle(fontSize: 18.0, color: Colors.black),
),
),
home: new MyHomePage(),
),
);
}
class MyHomePage extends StatefulWidget {
@override
_MyHomePageState createState() => new _MyHomePageState();
}
class _MyHomePageState extends State<MyHomePage> {
List<Color> color = [
Colors.white,
Colors.black,
Colors.pink,
Colors.blue,
Colors.red,
Colors.yellow,
Colors.orange,
Colors.green,
Colors.cyan,
Colors.purple,
Colors.brown,
Colors.indigo,
Colors.teal,
Colors.grey,
];
Color _pencolor = Colors.white;
Color _canvasclr = Colors.black;
bool ispen = true;
Color bc = Colors.black54;
List<Offset> points = List<Offset>();
GlobalKey<ScaffoldState> _skey = GlobalKey<ScaffoldState>();
int cchanged = change[1];
static var change = [1, 2, 0];
@override
Widget build(BuildContext context) {
MyPainter _painter = MyPainter(color: cchanged, canvasp: points, changed: color.indexOf(_pencolor));
_painter.addListener(() {
print('hello');
});
return new Scaffold(
resizeToAvoidBottomPadding: true,
key: _skey,
appBar: AppBar(
backgroundColor: bc,
elevation: 0.0,
title: Text(
'Draw',
style: Theme.of(context).textTheme.display1,
),
centerTitle: true,
actions: <Widget>[
IconButton(
icon: Icon(Icons.content_paste),
onPressed: () {
_skey.currentState.showSnackBar(SnackBar(
backgroundColor: Colors.transparent,
content: Center(
child: Text(
'Choose Canvas Color',
textScaleFactor: 0.7,
style: Theme.of(context).textTheme.display3,
))));
ispen = false;
}),
IconButton(
icon: Icon(Icons.edit),
onPressed: () {
_skey.currentState.showSnackBar(SnackBar(
content: Center(
child:
Text('Choose Pen Color', textScaleFactor: 0.7, style: Theme.of(context).textTheme.display3)),
backgroundColor: Colors.transparent,
));
ispen = true;
})
],
),
drawer: Drawer(
child: ListView(
children: <Widget>[
ListTile(
title: Text('Create Canvas'),
leading: Icon(Icons.add),
onTap: () {
//TODO
},
),
ListTile(
title: Text('Connect to Canvas'),
leading: Icon(Icons.compare_arrows),
onTap: () {
//TODO
},
)
],
),
),
body: Container(
color: bc,
child: Column(
children: <Widget>[
Expanded(
child: Padding(
padding: const EdgeInsets.only(left: 8.0, right: 8.0, top: 10.0),
child: ClipRRect(
child: Container(
color: _canvasclr,
child: GestureDetector(
onPanStart: (DragStartDetails d) {
points.add(Offset(d.globalPosition.dx, d.globalPosition.dy - 100));
cchanged = change[2];
print('${d.globalPosition},$points');
setState(() {});
},
onPanUpdate: (DragUpdateDetails d) {
points.add(Offset(d.globalPosition.dx, d.globalPosition.dy - 100));
print('${d.globalPosition}');
cchanged = change[0];
setState(() {});
},
child: CustomPaint(
isComplex: true,
willChange: false,
child: Container(),
painter: _painter,
),
),
),
borderRadius: BorderRadius.circular(25.0),
),
),
),
Container(
height: 75.0,
child: ListView.builder(
scrollDirection: Axis.horizontal,
shrinkWrap: true,
itemCount: color.length,
itemBuilder: (BuildContext context, int index) {
return InkWell(
splashColor: Colors.white,
onTap: () {
ispen ? _pencolor = color[index] : _canvasclr = color[index];
setState(() {});
},
child: Padding(
padding: const EdgeInsets.all(12.0),
child: CircleAvatar(
backgroundColor: color[index],
),
),
);
},
),
),
],
),
),
);
}
}
class MyPainter extends CustomPainter {
final int color;
final List<Offset> canvasp;
Paint p = Paint();
List<Paint> _paint = [
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.white,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.black,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.pink,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.blue,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.red,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.yellow,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.orange,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.green,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.cyan,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.purple,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.brown,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.indigo,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.teal,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.grey,
];
final int changed;
MyPainter({
this.color,
this.canvasp,
this.changed,
});
@override
void paint(Canvas canvas, Size size) {
print('painting ....... $canvasp');
for (int i = 0; i < canvasp.length; i++) {
canvas.drawCircle(canvasp[i], 10.0, _paint[color]);
}
}
@override
bool shouldRepaint(MyPainter oldDelegate) {
return oldDelegate.canvasp.length != canvasp.length;
}
}
This implementation shows an app that I am trying to build to draw on a canvas.
However, I can't get it to to draw as I touch the screen. Instead, it repaints only when I change color in the inkwell - but not when it is writing or when events are recieved in the GestureDetector.
An answer in the form of a change to my existing code would be appreciated.
Upvotes: 0
Views: 1563
Reputation: 40433
I'm going to preface this by saying please don't take anything I say in this answer personally - I'm aware of the be nice
efforts that StackOverflow has been trying to implement. But I'm also going to say this: you need to look into cleaning up your code. This isn't a personal attack, just some words from someone who wants to help you improve as a programmer.
You should definitely be breaking the logic into smaller chunks - a general rule is that if your build function takes up more than a page vertically, it's probably doing too much. This is true for both you as the programmer, but also for the performance of the application as flutter uses a state-based mechanism for rebuilding that is impacted by large build functions.
Another couple of hints are:
Don't use GlobalKey if at all possible. Your AppBar should be encapsulated in its own widget, or at the very least built with a Builder so that it receives a context which includes the scaffold. Then you can use Scaffold.of(....)
rather than the global key.
If you are going to use a static var like your static var change = [1, 2, 0]
, you'd probably be better off using an enum - especially without a documentation comment that means nothing to someone else looking at your code (i.e. me =D). Using an enum will make your code more readable. Also, please consider naming things differently - cchanged
means nothing to anyone but yourself (and possibly not even you if you step away for a couple of weeks).
The Colour list in your painter and widget is a coding bug waiting to happen. Building Paint objects is cheap so long as you don't do it in a loop - just instantiate a new one in the build function rather than keeping a list, and pass the colour in directly! Or at the very very least, make an enum
called Palette or something and use that to make the decisions rather than a list - at least that way if you use a switch the dart analyzer will help make sure you cater to each possible option.
This is a repeat of the second paragraph, but it's important enough to repeat... break your class into more Stateful or Stateless widgets... or at the very very least break your build function up into multiple functions. This will make things less complicated, easier to understand, and more performant. An example of why is that every time you draw one dot on your canvas, you're probably redrawing the title bar, bottom bar, and everything else you see on the screen at the same time simply because it's all in one widget! Your statefulwidget should only build the visual part that is stateful!
When using setState
, actually set the state within the setState
function. Right now, things will work if you don't... but it's not as semantically clear what you're doing and it could very easily break in the future if the flutter devs make a change to how setState works.
Because you're using Center
in your snackbars, they're expanding to take the entire screen. I'm fairly sure this isn't want you want to do, so add heightFactor = 1.0
so that they don't expand vertically. I'll leave it to you to decide if you want to continue to use this a visual mechanism for changing which colour you're changing - I personally think it's not great as it covers up the area you actually use to select the colour...
I'm going to guess that this is just a quick app put together for fun, and that you're in the process of learning flutter and/or coding. But take it from a professional developer & occaisional hiring manager - spending an extra couple minutes to make sure your code is readable and easy to understand will save you time in the long run, and that writing clean code even on throwaway projects will help you to develop good habits that will help you to become a better programmer. Also - when I'm checking up on a possible hire, if I see really messy code in one of their public github repositories, that's one more reason to not give them an interview. I know not everything you do on github is meant for the public, but code cleanliness is an easy skill to develop, doesn't take all that long, and says a lot about you as a programmer!
And one last thing - demanding that your question is answered in one particular way is a little bit rude considering we on stackoverflow are doing this for free because we want to help the community. I've softened the tone in your question, but keep that in mind because being polite ==> better answers!
Now on to your actual problem - it all boils down to this statement:
@override
bool shouldRepaint(MyPainter oldDelegate) {
return oldDelegate.canvasp.length != canvasp.length;
}
In dart (and many other programming languages, but not all), comparing a list with != or == only does what I'd call a 'shallow compare'; that is, it only checks whether the list is the same list. Because you're adding a new point to the same list each time you detect a gesture but not re-creating the list, the proceeding statement always returns false and therefore your canvas never re-draws.
The simplest way to resolve this is to copy the list into a new list each time you add something - that way the lists will compare as different. However that's not an especially great answer if you're dealing with a lot of elements which you will likely be here.
What I suggest instead is using a counter that you use to keep track of each time you change the list. I've made the change in some code below - I call it _revision
. Each time you add to the list, also increment _revision
and pass that through to the canvas. In your canvas, you just have to check whether the revision is the same. I'd suggest making a method that does the incrementing for you as well as making sure that setState is called appropriately.
import 'package:flutter/material.dart';
main() {
runApp(
new MaterialApp(
title: 'Flutter Database Test',
theme: ThemeData(
textTheme: TextTheme(
display1: TextStyle(fontSize: 24.0, color: Colors.white),
display2: TextStyle(fontSize: 24.0, color: Colors.grey),
display3: TextStyle(fontSize: 18.0, color: Colors.black),
),
),
home: new MyHomePage(),
),
);
}
class MyHomePage extends StatefulWidget {
@override
_MyHomePageState createState() => new _MyHomePageState();
}
class _MyHomePageState extends State<MyHomePage> {
List<Color> color = [
Colors.white,
Colors.black,
Colors.pink,
Colors.blue,
Colors.red,
Colors.yellow,
Colors.orange,
Colors.green,
Colors.cyan,
Colors.purple,
Colors.brown,
Colors.indigo,
Colors.teal,
Colors.grey,
];
Color _pencolor = Colors.white;
Color _canvasclr = Colors.black;
bool ispen = true;
Color bc = Colors.black54;
List<Offset> points = List<Offset>();
GlobalKey<ScaffoldState> _skey = GlobalKey<ScaffoldState>();
int cchanged = change[1];
int _revision = 0;
static var change = [1, 2, 0];
@override
Widget build(BuildContext context) {
MyPainter _painter =
MyPainter(color: color.indexOf(_pencolor), canvasp: points, changed: cchanged, revision: _revision);
_painter.addListener(() {
print('hello');
});
return new Scaffold(
resizeToAvoidBottomPadding: true,
key: _skey,
appBar: AppBar(
backgroundColor: bc,
elevation: 0.0,
title: Text(
'Draw',
style: Theme.of(context).textTheme.display1,
),
centerTitle: true,
actions: <Widget>[
IconButton(
icon: Icon(Icons.content_paste),
onPressed: !ispen
? null
: () {
_skey.currentState.showSnackBar(
SnackBar(
backgroundColor: Colors.transparent,
content: Center(
heightFactor: 1.0,
child: Text(
'Choose Canvas Color',
textScaleFactor: 0.7,
style: Theme.of(context).textTheme.display3,
),
),
),
);
setState(() {
ispen = false;
});
},
),
IconButton(
icon: Icon(Icons.edit),
onPressed: ispen
? null
: () {
_skey.currentState.showSnackBar(
SnackBar(
content: Center(
heightFactor: 1.0,
child: Text('Choose Pen Color',
textScaleFactor: 0.7, style: Theme.of(context).textTheme.display3)),
backgroundColor: Colors.transparent,
),
);
setState(() {
ispen = true;
});
},
)
],
),
drawer: Drawer(
child: ListView(
children: <Widget>[
ListTile(
title: Text('Create Canvas'),
leading: Icon(Icons.add),
onTap: () {
//TODO
},
),
ListTile(
title: Text('Connect to Canvas'),
leading: Icon(Icons.compare_arrows),
onTap: () {
//TODO
},
)
],
),
),
body: Container(
color: bc,
child: Column(
children: <Widget>[
Expanded(
child: Padding(
padding: const EdgeInsets.only(left: 8.0, right: 8.0, top: 10.0),
child: ClipRRect(
child: Container(
color: _canvasclr,
child: GestureDetector(
onPanStart: (DragStartDetails d) {
setState(() {
points.add(Offset(d.globalPosition.dx, d.globalPosition.dy - 100));
cchanged = change[2];
_revision++;
});
print('${d.globalPosition},$points');
},
onPanUpdate: (DragUpdateDetails d) {
print('${d.globalPosition}');
setState(() {
points.add(Offset(d.globalPosition.dx, d.globalPosition.dy - 100));
cchanged = change[0];
_revision++;
});
},
child: CustomPaint(
isComplex: true,
willChange: false,
child: Container(),
painter: _painter,
),
),
),
borderRadius: BorderRadius.circular(25.0),
),
),
),
Container(
height: 75.0,
child: ListView.builder(
scrollDirection: Axis.horizontal,
shrinkWrap: true,
itemCount: color.length,
itemBuilder: (BuildContext context, int index) {
return InkWell(
splashColor: Colors.white,
onTap: () {
setState(() {
ispen ? (_pencolor = color[index]) : (_canvasclr = color[index]);
});
},
child: Padding(
padding: const EdgeInsets.all(12.0),
child: CircleAvatar(
backgroundColor: color[index],
),
),
);
},
),
),
],
),
),
);
}
}
class MyPainter extends CustomPainter {
final int color;
final List<Offset> canvasp;
final int revision;
Paint p = Paint();
List<Paint> _paint = [
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.white,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.black,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.pink,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.blue,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.red,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.yellow,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.orange,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.green,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.cyan,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.purple,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.brown,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.indigo,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.teal,
Paint()
..strokeCap = StrokeCap.round
..strokeJoin = StrokeJoin.round
..strokeWidth = 2.0
..color = Colors.grey,
];
final int changed;
MyPainter({this.color, this.canvasp, this.changed, this.revision});
@override
void paint(Canvas canvas, Size size) {
print('painting ....... $canvasp');
for (int i = 0; i < canvasp.length; i++) {
canvas.drawCircle(canvasp[i], 10.0, _paint[color]);
}
}
@override
bool shouldRepaint(MyPainter oldDelegate) {
return oldDelegate.revision != revision;
}
}
I added in the revision parameter and did a couple other small fixes. Your code still needs a lot of refactoring, but this should at least make it work. And please keep my other comments in mind =).
Also - take a look at this question & answer as it does something similar and could help with saving the resulting drawing if that's something you want to do eventually!
Upvotes: 6