New issue
 Projects > js > Issues > Bug #4392

director.getOpenGL veiw js binding has bug

Bug #4392 [Closed]
hohohmm 2014-03-16 08:26 . Updated over 2 years ago

jsb_cocos2dx_auto.cpp : 41218
cocos2d::GLViewProtocol* cobj = (cocos2d::GLViewProtocol *)(proxy ? proxy->ptr : NULL);
produces incorrect cobj that cannot invoke any GLViewProtocol functions.
If I manually cast it to GLView*, it appears to work correctly. Not sure what's going on.
My version is the newly launched RC0

context code:

bool js_cocos2dx_GLViewProtocol_setDesignResolutionSize(JSContext *cx, uint32_t argc, jsval *vp)
{
    jsval *argv = JS_ARGV(cx, vp);
    bool ok = true;
    JSObject *obj = JS_THIS_OBJECT(cx, vp);
    js_proxy_t *proxy = jsb_get_js_proxy(obj);
    cocos2d::GLViewProtocol* cobj = (cocos2d::GLViewProtocol *)(proxy ? proxy->ptr : NULL);
    JSB_PRECONDITION2( cobj, cx, false, "js_cocos2dx_GLViewProtocol_setDesignResolutionSize : Invalid Native Object");
    if (argc == 3) {
        double arg0;
        double arg1;
        ResolutionPolicy arg2;
        ok &= JS::ToNumber( cx, JS::RootedValue(cx, argv[0]), &arg0);
        ok &= JS::ToNumber( cx, JS::RootedValue(cx, argv[1]), &arg1);
        ok &= jsval_to_int32(cx, argv[2], (int32_t *)&arg2);
        JSB_PRECONDITION2(ok, cx, false, "js_cocos2dx_GLViewProtocol_setDesignResolutionSize : Error processing arguments");
        cocos2d::Size size = cobj->getFrameSize();
        std::string xx = cobj->getViewName();
        printf("%s\n", xx.c_str());
        printf("%f %f\n", size.width, size.height);
        cobj->setDesignResolutionSize(arg0, arg1, arg2);
        JS_SET_RVAL(cx, vp, JSVAL_VOID);
        return true;
    }

    JS_ReportError(cx, "js_cocos2dx_GLViewProtocol_setDesignResolutionSize : wrong number of arguments: %d, was expecting %d", argc, 3);
    return false;
hohohmm 2014-03-16 15:21

The reason for this bug is kinda tricky.
director.getGLView() returns a pointer to GLView, which multi-inherits Ref and GLViewProtocol. Neither parent class is pure virtual, and as a result when you C-cast to GLViewProtocol, the result should not be a simple reinterpretation of the pointer, since the memory layout of GLViewProtocol is not 0 offset from GLView pointer. This appears to be quite SERIOUS and a mistake that should not be made in decent C++. Any call to s cc.director.getGLView().* will fault or do nothing. Please fix asap.

zhangxm 2014-03-17 02:59

@hohohmm
You are right. C-cast is dangerous here. We should use dynamic_cast instead.
We use C-cast just want to make it speed up, but haven't noticed multiple inheritance.

Good catch.
Thanks for feedback.

zhangxm 2014-03-17 03:00
  • Priority changed from Low to High
  • Target version set to v3.0 beta
hohohmm 2014-03-17 03:14

It's actually a systematic risk in all multi-inheriting classes.
Using plain dynamic_cast here probably would not work either, since dynamic_cast does not cast void*, which is proxy->ptr. It's probably okay to change js_proxy to simply store Ref* instead of void*, but then I noticed hand-written classes like WebSocket and SAXParser do not follow 2dx inheritance. I need a way to get around this fast. Hopefully the dev team could help a bit here.

hohohmm 2014-03-17 03:59

Btw, this issue almost renders 2dx unstable to a point that it's useless. Hopefully we could get a hotfix for this.

hohohmm 2014-03-17 05:18

Actually after merging the RC0 tag again, this issue appears to have gone away. The inheritance order is swapped for Ref and GLViewProtocol, and that does hide this issue. To be honest, it seems a very dangerous fix though, not really addressing this issue.

zhangxm 2014-03-17 06:13

Yep, we need to do more research for this issue. Because we want to avoid dynamic_cast as possible.

pandamicro 2014-05-23 17:40
  • Target version changed from v3.0 beta to v3.0 rc
pandamicro 2014-07-04 08:48
  • Target version changed from v3.0 rc to v3.0
pandamicro 2014-08-19 17:31

This have been fixed by refactoring getGLView to

inline GLView* getOpenGLView() { return _openGLView; }

And the generated code for getOpenGLView in JSB is now:

bool js_cocos2dx_Director_getOpenGLView(JSContext *cx, uint32_t argc, jsval *vp)
{
    JSObject *obj = JS_THIS_OBJECT(cx, vp);
    js_proxy_t *proxy = jsb_get_js_proxy(obj);
    cocos2d::Director* cobj = (cocos2d::Director *)(proxy ? proxy->ptr : NULL);
    JSB_PRECONDITION2( cobj, cx, false, "js_cocos2dx_Director_getOpenGLView : Invalid Native Object");
    if (argc == 0) {
        cocos2d::GLView* ret = cobj->getOpenGLView();
        jsval jsret = JSVAL_NULL;
        do {
            if (ret) {
                js_proxy_t *jsProxy = js_get_or_create_proxy(cx, (cocos2d::GLView*)ret);
                jsret = OBJECT_TO_JSVAL(jsProxy->obj);
            } else {
                jsret = JSVAL_NULL;
            }
        } while (0);
        JS_SET_RVAL(cx, vp, jsret);
        return true;
    }

    JS_ReportError(cx, "js_cocos2dx_Director_getOpenGLView : wrong number of arguments: %d, was expecting %d", argc, 0);
    return false;
}

So there won't be any problem of casting object

pandamicro 2014-08-19 17:32
  • Status changed from New to Closed
  • % Done changed from 0 to 100

Atom PDF

Status:Closed
Start date:2014-03-16
Priority:High
Due date:
Assignee:-
% Done:

100%

Category:-
Target version:v3.0

Sign up for our newsletter to keep up with the latest developments, releases and updates for Cocos2d-x.