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

Updating from 0.3.3 to 0.4.1, it breaks my charts (LineChart and PieChart) #100

Closed
jamesblasco opened this issue Nov 9, 2019 · 13 comments · Fixed by #110
Closed

Updating from 0.3.3 to 0.4.1, it breaks my charts (LineChart and PieChart) #100

jamesblasco opened this issue Nov 9, 2019 · 13 comments · Fixed by #110
Labels
bug Something isn't working

Comments

@jamesblasco
Copy link
Contributor

Given a size to the father container of my chart it throws this error.

The following assertion was thrown building PieChart(duration: 150ms, dirty, dependencies: [MediaQuery], state: PieChartState#f59ef(ticker active)):
RenderBox was not laid out: RenderCustomPaint#99bfc NEEDS-LAYOUT NEEDS-PAINT 'package:flutter/src/rendering/box.dart': Failed assertion: line 1687 pos 12: 'hasSize'

@imaNNeo
Copy link
Owner

imaNNeo commented Nov 9, 2019

I will check,
Thanks

@jamesblasco
Copy link
Contributor Author

I will try to research more about it and give a proper example. It happens when i resize de view for example when i rotate the device

@imaNNeo
Copy link
Owner

imaNNeo commented Nov 9, 2019

Ok prepare a reproducers sample, It helps me to debug it faster.

@jamesblasco
Copy link
Contributor Author

jamesblasco commented Nov 9, 2019

I found when does it happen but don't know how to solve it.

It happens when the chart is inside a LayoutBuilder widget.
I replaced the _MyHomePageState of the fl_chart example with the following code and it throws the error.

class _MyHomePageState extends State<MyHomePage> {
 
  @override
  Widget build(BuildContext context) {
    return LayoutBuilder(
        builder: (BuildContext context, constraints) => Center(
            child: FractionallySizedBox(
                heightFactor: 0.5,
                widthFactor: 0.5,
                child: Scaffold(
                  body: SafeArea(
                    child: PageView(
                      children: <Widget>[
                        LineChartPage(),
                        BarChartPage(),
                        BarChartPage2(),
                        PieChartPage(),
                        LineChartPage2(),
                        LineChartPage3(),
                        LineChartPage4(),
                      ],
                    ),
                  ),
                ))));
  }
}

@imaNNeo
Copy link
Owner

imaNNeo commented Nov 10, 2019

Dude you are trying to show the whole page with a 0.5 scale factor.
Is it intentionally?

@jamesblasco
Copy link
Contributor Author

Yes, sorry i was testing it on ipad and i wanted to check the resize when rotating. But I think for the example is not needed

@jamesblasco
Copy link
Contributor Author

jamesblasco commented Nov 10, 2019

The error happens in containerRenderBox.size when the chart is a child of an upper tree that contains a LayoutBuilder.

My usecase of LayoutBuilder:
In my project I use LayoutBuilder to display different layout for this cases portrait or landscape and smartphone or ipad.

@imaNNeo
Copy link
Owner

imaNNeo commented Nov 10, 2019

Run it with scale factor 1.0,
It works perfectly

@jamesblasco
Copy link
Contributor Author

jamesblasco commented Nov 10, 2019

Not to me. It is like this with all the charts

ezgif-6-156c10e6c7c0

@jamesblasco
Copy link
Contributor Author

jamesblasco commented Nov 10, 2019

I think it is related to this :

widgets > BuildContext > findRenderObject

This method will only return a valid result after the build phase is complete. It is therefore not valid to call this from a build method. It should only be called from interaction event handlers (e.g. gesture callbacks) or layout or paint callbacks.

If the render object is a RenderBox, which is the common case, then the size of the render object can be obtained from the size getter. This is only valid after the layout phase, and should therefore only be examined from paint callbacks or interaction event handlers (e.g. gesture callbacks).

In this part of the code the RenderBox comes from the findRenderObject(). So probably when rotating it tries to get the size of the render box while the layout is being built.

Size _getChartSize() {
    if (_chartKey.currentContext != null) {
      final RenderBox containerRenderBox = _chartKey.currentContext.findRenderObject();
      return containerRenderBox.size;
    } else {
      return getDefaultSize(context);
    }
  }

@jamesblasco
Copy link
Contributor Author

jamesblasco commented Nov 10, 2019

I replaced containerRenderBox.size to containerRenderBox.constraints.biggest and it works.

I don't understand much the getDefaultSize() and why it is a square 70% of the screen

@imaNNeo
Copy link
Owner

imaNNeo commented Nov 10, 2019

Good catch!
I appreciate it, and please make a PR and help to fix it, it's valuable for the community.
getDefaultSize() is just return a default size (when there is no constraint for the chart, for example when you try to show it directly as a centered widget).
but the best case is to size it yourself.
(for example, you can show a Container that fills the screen (default size), but sometimes you need to set the size manually).

@ica4c
Copy link
Contributor

ica4c commented Nov 16, 2019

Good evening. I've tried @jamesblasco solution over a LayoutBuilder and it didn't work. Got same NEEDS-LAYOUT NEEDS-PAINT debug message. What I found though when I comment out the chartKey at CustomPaint of e.g LineChart class everything start to work as expected I haven't spotted any differences in behavior too. What it actually do? Is it crucial in this context? Might be a dumb question, sorry in advance)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants