cutelyaware / magiccube4d

Automatically exported from code.google.com/p/magiccube4d
Other
71 stars 16 forks source link

possible excessive repainting on mouse motion due to errant semicolon #136

Closed donhatch closed 4 years ago

donhatch commented 4 years ago

OpenSDK javac warns about the semicolon after the "if" clause in MC4DView.java line 264:

        @Override
        public void mouseMoved(MouseEvent me) {
            super.mouseMoved(me);
            if(puzzleManager != null) {
                if(!isInMotion() && puzzleManager.updateStickerHighlighting(me.getX(), me.getY(), getSlicemask(), me.isControlDown()))
                    ;
                repaint();
                return;
            }
        }

It's possible that the semicolon shouldn't be there, which would mean repaint() is getting called when it shouldn't be (that is, on every mouse motion, even when the picture isn't changing).

I'm thinking that removing these excessive repaints could greatly improve interaction/responsiveness, especially on bigger puzzles like the hypermegaminx, in which every at-rest paint takes 2 seconds due to antialiasing.

donhatch commented 4 years ago

Here are some missing repaint issues I have noticed. These were observable even before Melinda's fix, but the fix makes them often linger longer.

repaint() seems to be missing:

  1. when user changes a color (sky, ground, outlines)
  2. when Help->Debugging is turned on or off
cutelyaware commented 4 years ago

OK, I fixed those new repaint cases. Turns out it also allows the users to now see their color adjustments in real-time. Check it out:

  1. Click a color button such as Outlines. (Make sure it's also enabled)
  2. In the color dialog that pops up, select one of the the other tabs such as HSV.
  3. Drag one of the sliders around.
donhatch commented 4 years ago

Thanks!

Regarding realtime color adjustment-- that's certainly worlds better, as long as the puzzle is small and/or antialiasing is off. However, try it in hypermegaminx with "Allow Antialiasing" enabled (which is the default, I believe); it seems pretty painful to me. See what you think.