pissang / clay-viewer

3D model viewer with high quality rendering and glTF2.0/GLB export
https://pissang.github.io/clay-viewer/editor/
BSD 3-Clause "New" or "Revised" License
771 stars 97 forks source link

Which is LRU or LRUCache correct? #9

Closed cx20 closed 6 years ago

cx20 commented 6 years ago

When trying to build the latest version, LRUCache function seems to be undefined. https://github.com/pissang/qtek-model-viewer/blob/e9400a4c5ead79f70ed63bb3e6a28b689724e8db/src/graphic/helper.js#L105

In QMV.js (2017.11.12) in the dist folder of this repository, the LRU function is used. Which is LRU or LRUCache correct?

pissang commented 6 years ago

Seems the LRUCache is not imported correctly. Try npm install zrender to update zrender version.

cx20 commented 6 years ago

Hmm. I updated zrender with npm install zrender, however LRUCache seems to be undefined.

pissang commented 6 years ago

Weird, I just removed the dependency which is added today. Not sure if this commit https://github.com/pissang/qtek-model-viewer/commit/7081142b680335d54ba296efbe0970f15ad9eca9 can fix your issue. Also, the build files are updated.

cx20 commented 6 years ago

I downloaded the latest repository (https://github.com/pissang/qtek-model-viewer/archive/master.zip) and tried npm install and npm run build again However, it did not resolve LRUCache's problem :(

cx20 commented 6 years ago

BTW, the problem with LRUCache is an event from several days ago. So, I think that it is not that today's fix is affecting.

pissang commented 6 years ago

Understand. LRU in dist folder is generated by rollup, which is the name used in the LRU module. I think it's a side effect or feature of scope hoisting. Do you get any issue on this LRUCache stuff?

cx20 commented 6 years ago

Do you get any issue on this LRUCache stuff?

When LRUCache uses the generated version, the following error is displayed when displaying the glTF model. (I depend on Google Translate, so I am sorry if I do not seem to understand the question correctly.)

QMV.js:27732 
Uncaught ReferenceError: LRUCache is not defined
    at Object.helper.loadTexture (QMV.js:27732)
    at Object.helper.createAmbientCubemap (QMV.js:27841)
    at SceneHelper.updateAmbientCubemapLight (QMV.js:29692)
    at Viewer.setAmbientCubemapLight (QMV.js:32757)
    at Task.<anonymous> (index.js:94)
    at Task.trigger (QMV.js:146)
    at Viewer.<anonymous> (QMV.js:32478)
    at Viewer.wrapper (QMV.js:211)
    at sub.trigger (QMV.js:146)
    at afterLoadBuffer (QMV.js:18506)
pissang commented 6 years ago

Seems your build is different from mine. What rollup version do you use? I've encountered a similar issue when using rollup >=0.51.0

cx20 commented 6 years ago

My version of rollup is below.

C:\home\edu\javascript\library\qtek-model-viewer-master>rollup -version
rollup version 0.51.2
cx20 commented 6 years ago

I updated rollup to the latest version (0.51.5) and it seems the problem has been resolved. LRU is now used instead of LRUCache.

    var textureCache = app.__textureCache = app.__textureCache || new LRU(20);
pissang commented 6 years ago

Well, I also have this issue when using 0.51.2. It is fixed in latest version 0.51.5. And earlier version 0.50.0 also don't have this problem.

pissang commented 6 years ago

@cx20 Great! Closing this issue.