soegaard / whalesong

Whalesong: Racket to JavaScript compiler
http://hashcollision.org/whalesong
145 stars 14 forks source link

place-image functions doesn't work #10

Closed vishesh closed 9 years ago

vishesh commented 9 years ago

place-image and related function don't work. For example the image-library-example.rkt in examples fails.

Tracing the problem, I figured in kernel.js:makeSceneImage function the value of children before calling SceneImage constructor and inside SceneImage constructor are different. I'm compiling on Racket 6.1.1 on Linux and testing on latest Chrome and Firefox browser.

thanks.

soegaard commented 9 years ago

Thanks for the bug report.

@schanzer Do you have an idea?

vishesh commented 9 years ago

Its problem with image/kernel.js:makeSceneImage calling SceneImage constructor with insufficient arguments (missing color). Seems like signature was changed, since some functions image/js-impl.js are using more arguments. I'm working on fixing it.

soegaard commented 9 years ago

Fix by vishesh fixed this

stchang commented 9 years ago

Should makeSceneImage (and its call sites) also be updated to consume a color?

For example, it seems that color c should be passed in here? https://github.com/soegaard/whalesong/blob/master/whalesong/image/private/js-impl.js#L924

soegaard commented 9 years ago

SceneImage supports a color already, so I think the answer is yes.

https://github.com/soegaard/whalesong/blob/master/whalesong/image/private/kernel.js#L291

I tried a quick search in the code to see how many instances of makeSceneImage there are, but I found out that you can't search a forked repository :-(

stchang commented 9 years ago

Ok thanks. That's what I thought as well.

Vishesh, can you make those changes? (Vishesh is working with me to explore using whalesong in an htdp course.) And can you also fix the signature of SceneImage :) at this line https://github.com/soegaard/whalesong/blob/master/whalesong/image/private/kernel.js#L291

vishesh commented 9 years ago

Yes. I will make the changes :)

soegaard commented 9 years ago

Interesting project. I'd love to hear more about it at some point.

schanzer commented 9 years ago

Sorry, just checking in now. @vishesh got to it before I could. Good catch!

Now if only someone would implement last-picture....;)

vishesh commented 9 years ago

last-picture used by stop-when I believe? We definitely need that. I will see what I can do. Did you try and face some problem?

schanzer commented 9 years ago

There's an enormous list of things on my software development pile, but these days my time is overwhelmingly devoted to training teachers, answering questions, and writing curriculum. I just don't have the bandwidth to take this on. :(

stchang commented 9 years ago

@schanzer I believe @vishesh was merely asking if someone tried to implement last-picture and ran into a problem? Or if it's a "to do" that no one's gotten to yet? That knowledge would help guide how we approach the problem. It sounds like it's the latter so we'll take a shot at implementing it.

schanzer commented 9 years ago

Sorry, I should have been more clear - what I mean is "due to the enormous list..." I haven't gotten around to it yet.

stchang commented 9 years ago

Great thanks. We'll see if we can help out.