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

Something wrong with pop() [WEBGL constant is another issue] #3

Closed
villares opened this issue May 2, 2019 · 9 comments
Closed

Something wrong with pop() [WEBGL constant is another issue] #3

villares opened this issue May 2, 2019 · 9 comments
Milestone

Comments

@villares
Copy link
Contributor

villares commented May 2, 2019

I'm sure we'll have 3D at some point :)

So let's put a small test example here:

from pyp5js import *

def setup():
    createCanvas(600, 600, WEBGL)
    
def draw():
  background(200)
  translate(-240, -100, 0)
  normalMaterial()
  push()
  rotateZ(frameCount * 0.01)
  rotateX(frameCount * 0.01)
  rotateY(frameCount * 0.01)
  box(50, 70, 100)
  pop()

start_p5(setup, draw)

I guess this is a bit early... but, who knows?

@villares
Copy link
Contributor Author

villares commented May 3, 2019

Using _P5_INSTANCE.WEBGL I get this:
TypeError: _P5_INSTANCE.py_pop is not a function

@berinhard berinhard added the bug label May 4, 2019
@villares villares changed the title Maybe the WEBGL constant for createCanvas() is missing Something wrong with pop() [WEBGL constant is another issue] May 5, 2019
@berinhard
Copy link
Owner

I got the same error here, but at least the 3D is working for now WEBGL.

@berinhard berinhard reopened this May 5, 2019
@berinhard
Copy link
Owner

berinhard commented May 6, 2019

Updates to ongoing investigation:

The issue here is related to how Transcrypt converts the pop function from Python objects. With a more detailed version of the traceback, we can confirm that p5's pop was mapped to a new js function called py_pop:

Screenshot from 2019-05-06 08-36-20

Reading Transcrypt's source code, I was able to find the line where this mapping is defined. But I think it's a compile error, since our pop function is a regular one, defined by us, and pop functions only exist in Python via lists or dicts. Probably this will require a fix to Transcrypt.

@berinhard
Copy link
Owner

Transcrypt's __pragma__ seemed to deal with the problem. The following snippet in pyp5js/pytop5js.py worked for me:

def pop(*args):
    __pragma__ ('noalias', 'pop')
    _P5_INSTANCE.pop()
    __pragma__ ('alias', 'pop', 'py_pop')

@eduardostalinho
Copy link
Contributor

Based on https://www.transcrypt.org/docs/html/special_facilities.html#id2, I think there are more instructions with this issue..

@berinhard
Copy link
Owner

@eduardostalinho I think this is true only for methods from P5.js that matches with any of those aliases definition. I spent today's morning taking a look at them and it seemed that case only occurs with p5.pop function.

Anyway, if we have the same error in the future with other functions, we'll already know how to fix them. So, I'm closing this issue as solved! Thanks for your PR =]

berinhard pushed a commit that referenced this issue May 9, 2019
@berinhard berinhard added this to the Release 0.0.4 milestone May 9, 2019
@kylepollina
Copy link

kylepollina commented Feb 7, 2021

Hi all, I think this issue might have to open up again, or at least as a separate issue.

p5.Renderer objects also contain pop() and clear() methods that are renamed during the transcrypt stage. For example, a simple Python script like this:

from pyp5js import *

def setup():
    createCanvas(700, 700)

    gr = createGraphics(500, 500)
    gr.push()
    gr.pop()

will get compiled into this equivalent Javascript

export var setup = function () {
	if (arguments.length) {
		var __ilastarg0__ = arguments.length - 1;
		if (arguments [__ilastarg0__] && arguments [__ilastarg0__].hasOwnProperty ("__kwargtrans__")) {
			var __allkwargs0__ = arguments [__ilastarg0__--];
			for (var __attrib0__ in __allkwargs0__) {
			}
		}
	}
	else {
	}
	createCanvas (700, 700);
	var gr = createGraphics (500, 500);
	gr.push ();
	gr.py_pop ();
};

Which will result in the following error Uncaught TypeError: gr.py_pop is not a function. The same happens for the following:

gr = createGraphics(500, 500)
gr.clear()

gets transcrypted into

var gr = createGraphics (500, 500);
gr.push ();
gr.py_clear ();

Creates the error Uncaught TypeError: gr.py_clear is not a function.

I have tried this in the pyp5js.py file:

def createGraphics(*args):
    __pragma__('noalias', 'pop')
    graphics = _P5_INSTANCE.createGraphics(*args)
    __pragma__('alias', 'pop', 'py_pop')
    return graphics

but didn't fix anything. Do any of you have thoughts on how this might be avoided?

@kylepollina
Copy link

kylepollina commented Feb 7, 2021

Oh you know what, nevermind. I now understand I can do this instead in my python script and it will work as expected:

from pyp5js import *

def setup():
    createCanvas(700, 700)

    gr = createGraphics(500, 500)
    gr.push()
    __pragma__('noalias', 'pop')
    gr.pop()
    __pragma__('alias', 'pop', 'py_pop')

The pragma just needs to be in my code instead of in the pyp5js library. I can open up a pull request with additional documentation on this quirk.

@berinhard
Copy link
Owner

Thanks for figuring this out @kylepollina! Unfortunately pyodide support (#124) is not ready yet. Once we have this done, these transcrypt specific fixes won't be necessary anymore =]

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

No branches or pull requests

4 participants