kivy-garden / graph

Displays plots on a graph.
MIT License
50 stars 18 forks source link

BUG: To many elements in vertices deleted #19

Closed davidkleiven closed 4 years ago

davidkleiven commented 4 years ago

It looks like BarPlot deletes too many elements in the vertices array when the number of points is reduced. It looks like there should be 24 entries for each point. From quick tests the code crashes in its current form, and behaves correctly after the proposed change.

matham commented 4 years ago

This is not correct. You can see it in three lines under your change that we use 4 elements per point.

It would be helpful if you share some example code and resulting graphs to demonstrate the issue.

davidkleiven commented 4 years ago

@matham Sure here is a code

from kivy_garden.graph import Graph, BarPlot
from kivy.uix.boxlayout import BoxLayout
from kivy.uix.button import Button
from kivy.app import App

class TestApp(App):
    plot = None
    def build(self):
        b = BoxLayout(orientation='vertical')
        graph = Graph(xmin=-50, xmax=50, ymin=-1, ymax=1)
        self.plot = BarPlot()
        graph.add_plot(self.plot)

        self.plot.points = [(0, -0.2), (10, 0.6), (15, 1.0)]
        b.add_widget(graph)
        button = Button(text='Hello world', font_size=14)
        button.bind(on_press=self.replot)
        b.add_widget(button)
        return b

    def replot(self, inst):
        self.plot.points = [(-1, 0.5)]

TestApp().run()

if you click on the button it crashes. If you look in the code from line 1443 it reads

for k in range(point_len):
            p = points[k]
            x1 = x_px(p[0])
            x2 = x1 + bar_width
            y1 = ymin
            y2 = y_px(p[1])

            idx = k * 24
            # first triangle
            vert[idx] = x1
            vert[idx + 1] = y2
            vert[idx + 4] = x1
            vert[idx + 5] = y1
            vert[idx + 8] = x2
            vert[idx + 9] = y1
            # second triangle
            vert[idx + 12] = x1
            vert[idx + 13] = y2
            vert[idx + 16] = x2
            vert[idx + 17] = y2
            vert[idx + 20] = x2
            vert[idx + 21] = y1

so for each point idx is incremented by 24. Hence, as far as I can understand too many elements are deleted. In the example I initially had 3 points, when the number of points is lowered the following error appear.

Traceback (most recent call last):
   File "kivy_graph_error_in_delete.py", line 25, in <module>
     TestApp().run()
   File "/home/davidkleiven/.local/lib/python3.7/site-packages/kivy/app.py", line 855, in run
     runTouchApp()
   File "/home/davidkleiven/.local/lib/python3.7/site-packages/kivy/base.py", line 504, in runTouchApp
     EventLoop.window.mainloop()
   File "/home/davidkleiven/.local/lib/python3.7/site-packages/kivy/core/window/window_sdl2.py", line 747, in mainloop
     self._mainloop()
   File "/home/davidkleiven/.local/lib/python3.7/site-packages/kivy/core/window/window_sdl2.py", line 479, in _mainloop
     EventLoop.idle()
   File "/home/davidkleiven/.local/lib/python3.7/site-packages/kivy/base.py", line 339, in idle
     Clock.tick()
   File "/home/davidkleiven/.local/lib/python3.7/site-packages/kivy/clock.py", line 591, in tick
     self._process_events()
   File "kivy/_clock.pyx", line 384, in kivy._clock.CyClockBase._process_events
   File "kivy/_clock.pyx", line 414, in kivy._clock.CyClockBase._process_events
   File "kivy/_clock.pyx", line 412, in kivy._clock.CyClockBase._process_events
   File "kivy/_clock.pyx", line 167, in kivy._clock.ClockEvent.tick
   File "/home/davidkleiven/Documents/graph/kivy_garden/graph/__init__.py", line 1454, in draw
     vert[idx + 4] = x1
 IndexError: list assignment index out of range

which suggest that the vert array is too short. The above snippet from the source code kivy_garden/graph/init.py where idx is incremented by 24 for each point, strongly suggest that there are 24 items in vert for each point.

This can further be confirmed by editing the replot method in my example to

def replot(self, inst):
        print(len(self.plot._mesh.vertices))
        self.plot.points = [(-1, 0.5)]

which prints 72. Hence, there has to be 24 items in self.plot._mesh.vertices for each item in self.plot.points . @matham If you mean that this is not correct, can elaborate on why it should by only 4 despite the fact that idx is incremented by 24?

davidkleiven commented 4 years ago

Filed as bug report in issue #20

matham commented 4 years ago

Sorry, my bad. It indeed seems to use 6 points per bar.