search

lunduniversity / introprog-scalalib

Scala library with simple-to-use utilites for students of introductory programming. http://cs.lth.se/pgk/api
BSD 2-Clause "Simplified" License
60 stars 14 forks source link

Add rotation to PixelWindow.drawImage #34

Closed theolundqvist closed 3 years ago

theolundqvist commented 3 years ago

Thanks to @JoPoLu for the code.

bjornregnell commented 3 years ago

Thanks for this enhancement! I'm wondering how to interpret the angle param (perhaps improve doc comment?): Does 0 mean that nothing changes? And if so, we could avoid calling rotate if angle == 0 ? And if 0 means no change then very good, because then this does not break existing code thanks to the default param.

theolundqvist commented 3 years ago

Thanks for this enhancement! I'm wondering how to interpret the angle param (perhaps improve doc comment?): Does 0 mean that nothing changes? And if so, we could avoid calling rotate if angle == 0 ? And if 0 means no change then very good, because then this does not break existing code thanks to the default param.

If angle equals 0 then the image will not be rotated, if angle equals pi/2 then the image will be rotated 90 degrees clockwise. Would degrees or radians be better for simplicity and cohesion?

I could check if angle == 0 and then just do drawImage(img, x, y, width, height), would not need to use AffineTransform at all then.

bjornregnell commented 3 years ago

Yes it would be good if we could avoid allocation of affine transformation if it is not needed. But I guess it is needed if the width and height is changed? Could that also be checked? I think radians is best.

theolundqvist commented 3 years ago

Yes it would be good if we could avoid allocation of affine transformation if it is not needed. But I guess it is needed if the width and height is changed? Could that also be checked? I think radians is best.

Done

JoPoLu commented 3 years ago

I don't see why we would need an affinetransform if rotation = 0 other than the code would look cleaner if you do not have an if statement. I'm not able to figure out how changing drawImage to using affinetransform internally would break any old code. Don't think it lowers performance by a lot either considering that the normal drawImage also has to scale the Images. I did some tests and It resultet in this: (rotation was at 0 during said tests) It Took: 489 ms to complete; 100000 drawings using old/standard drawImage It Took: 412 ms to complete; 100000 drawings using AffineTransform drawImage (Note: the answers are within my margin of error, I was not able to do any more tests as I have not yet figure out how to allocate more Ram to the heap :) ) One might take more ram than the other, not sure Graphics2D might even be using Affinetransform internally when we call the standard version of drawImage(No affinetransform as an argument).

(Have not been able to find the source code for g2d.drawImage, guess this is the second best. drawImage Taken from: https://docs.oracle.com/javase/7/docs/api/java/awt/Graphics2D.html#drawImage(java.awt.image.BufferedImage,%20java.awt.image.BufferedImageOp,%20int,%20int)

Regarding radians or degrees I think that we should use Radians since all math. returns radians / takes in radians :)