-
Notifications
You must be signed in to change notification settings - Fork 228
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
next: Support Seamless Texture & Enhance Stone Quality #366
Conversation
Image stone = theme.getWhiteStone(new int[]{x, y}); | ||
g.drawImage(stone, centerX - stoneRadius, centerY - stoneRadius, stoneRadius * 2 + 1, stoneRadius * 2 + 1, null); | ||
// Enhance draw quality | ||
BufferedImage stone = theme.getWhiteStone(new int[]{x, y}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is copy pasted 4 times in this PR... Do you think you could refactor that method to avoid the duplication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, of course. I have changed and committed. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@OlivierBlanvillain |
/** | ||
* Draw scale smooth image, enhanced display quality | ||
*/ | ||
public void drawScaleSmoothImage(Graphics2D g, BufferedImage img, int x, int y, int width, int height, ImageObserver observer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code?
/** | ||
* Draw scale smooth image, enhanced display quality, less and faster than drawScaleSmoothImage | ||
*/ | ||
public void drawScaleImage(Graphics2D g, BufferedImage img, int x, int y, int width, int height, ImageObserver observer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also dead code??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the both functions are dead code, but the drawScaleSmoothImage can provide high quality scale draw, the drawScaleImage can provide better quality than the default and faster than the drawScaleSmoothImage, so I think it might be useful to keep it.
Of course, if it doesn't allow exists, I will remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think it might be useful to commit these functions even if they are unused, then please put the code inside a comment with an explanation of what it does differently that the one that's actually used...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have described & disabled the both functions.
g.drawImage(stone, centerX - stoneRadius, centerY - stoneRadius, stoneRadius * 2 + 1, stoneRadius * 2 + 1, null); | ||
// Enhance draw quality | ||
int size = stoneRadius * 2 + 1; | ||
g.drawImage(getScaleStone(stone, color, size, size), centerX - stoneRadius, centerY - stoneRadius, size, size, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there is also a lot of copy pasting here... For fancy-stones, I think the entire switch statement can be replaced by 6 lines:
if (uiConfig.getBoolean("fancy-stones")) {
boolean isBlack = color == BLACK || color == BLACK_GHOST || color == BLACK_CAPTURED;
boolean isGhost = color == BLACK_GHOST || color == WHITE_GHOST;
drawShadow(gShadow, centerX, centerY, isGhost);
Image stone = isBlack ? theme.getBlackStone(new int[]{x, y}) : theme.getWhiteStone(new int[]{x, y});
int size = stoneRadius * 2 + 1;
g.drawImage(getScaleStone(stone, color, size, size), centerX - stoneRadius, centerY - stoneRadius, size, size, null);
} else {
// code for non-fancy stones
}
By the way, do you know why do we keep two code paths here? Could we keep only fancy-stones
and remove the other one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you're right.
Because there are 'fancy-stone' settings in the configuration file, some people may like simple styles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have simplified code.
case BLACK_CAPTURED: | ||
case BLACK_GHOST: | ||
if (cachedBlackStoneImage == null || cachedBlackStoneImage.getWidth() != width || cachedBlackStoneImage.getHeight() != height) { | ||
stone = new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire block is copy pasted, could you please factor it out?
👍 |
#361