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

Add initial fix for npe #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aidenkeating
Copy link

Ignore the formatting changes, my editor auto-fixes stuff. I'll comment the main part that was causing the issue.

As a side note, depending on the image you use you'll notice the rectangles you add to pane might be off in your UI. This is because if you pick a large image, JavaFX is scaling it down in the image view. but your Rectangle coordinates are based on the full sized image.

Two ways to approach this:

  • Scale the rectangle coordinates with the ratio between the original image size and the displayed image size in JavaFX
  • Draw the rectangles to a BufferedImage (or any other Image class) and then update the ImageView with that image. Then the rectangles will be scaled too because they'll actually be drawn to the image.

@@ -255,8 +254,7 @@ private void createBirdBoxes() {
maxY = (maxY < yLoc) ? yLoc : maxY;
}
}
birdBoxes[index++] = new Rectangle(minX, minY, maxX - minX, maxY - minY);
pane.getChildren().addAll(birdBoxes[index++]);
pane.getChildren().addAll(new Rectangle(minX, minY, maxX - minX, maxY - minY));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't read the code too much. But from that it looks like this logic was adding a Rectangle to a list and then adding it as a child of the Pane. The list of Rectangle doesn't seem to be used. So just add the Rectangle directly.

Also, as a side note, index++ is done twice here. index++ changes index, so this code is essentially adding 2 to index every time it gets to this point. I assume this wasn't the intention?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant