kliment / Printrun

Pronterface, Pronsole, and Printcore - Pure Python 3d printing host software
GNU General Public License v3.0
2.36k stars 995 forks source link

p/excluder: Force line_scaler to return integers #1307

Closed rockstorm101 closed 1 year ago

rockstorm101 commented 1 year ago

Fix for #1306. If anyone can think of a more elegant solution, I'm not particularly proud of this one.

kliment commented 1 year ago

This looks fine to me. I don't see anything wrong with it. @hroncok just a sanity check, should this be handled on another layer?

hroncok commented 1 year ago

I wonder where are the floats coming from originally. Is it from self.p.scale, self.gcode_to_real or self.p.translate?

kliment commented 1 year ago

I suspect gcode_to_real

rockstorm101 commented 1 year ago

As far as I can tell, all initial orig argument, self.p.scale and self.gcode_to_real provide float numbers. See example values below. But I admit I don't fully understand how this Excluder window works exactly.

    def _line_scaler(self, orig):                        # orig: (117.6938, 112.5248, 139.1650, 137.5745)
        x0, y0 = self.gcode_to_real(orig[0], orig[1])    # x0, y0: 117.6938, 87.4751
        x0 = self.p.scale[0] * x0 + self.p.translate[0]  # p.scale:  [2.515, 2.515]
        y0 = self.p.scale[1] * y0 + self.p.translate[1]  # p.translate:  [0.0, 0.0]
        x1, y1 = self.gcode_to_real(orig[2], orig[3])    # x1, y1: 139.1650 62.4254
        x1 = self.p.scale[0] * x1 + self.p.translate[0]
        y1 = self.p.scale[1] * y1 + self.p.translate[1]
        width = max(x0, x1) - min(x0, x1) + 1
        height = max(y0, y1) - min(y0, y1) + 1
        return (min(x0, x1), min(y0, y1), width, height,)
kliment commented 1 year ago

Oh, we should probably convert to int in the return instead of the intermediate value

rockstorm101 commented 1 year ago

Something like this you mean?

         y1 = self.p.scale[1] * y1 + self.p.translate[1]
         width = max(x0, x1) - min(x0, x1) + 1
         height = max(y0, y1) - min(y0, y1) + 1
-        return (min(x0, x1), min(y0, y1), width, height,)
+        return tuple(map(int, (min(x0, x1), min(y0, y1), width, height)))

     def paint_selection(self, dc):
         dc = wx.GCDC(dc)

I did it on intermediate values because it was cleaner and the operations after that are just min, max or +1 so it won't (hopefully) affect the int type but the above is probably more "future-proof"

kliment commented 1 year ago

Yes exactly that.

kliment commented 1 year ago

Thanks!