jnsmalm / pixi3d

The 3D renderer for PixiJS. Seamless integration with 2D applications.
https://pixi3d.org
MIT License
759 stars 44 forks source link

Won't load glb anymore since pixi 6.4.2 #104

Closed bencresty closed 2 years ago

bencresty commented 2 years ago

Just posted this issue in the pixijs repo, but seeing now that the glTFAsset object is inside this project and probably the loader is extended from pixi3d to make importing glb's work, it could as well be a conflict/issue in pixi3d that's causing the loading of glb files to fail since pixi 64.2.

Please take a look at https://github.com/pixijs/pixijs/issues/8448

Thanks in advance!

(Using pixi3d version 1.4.1, so the now latest)

jnsmalm commented 2 years ago

Thanks! I have tested with your glb model and PixiJS 6.4.2 and from what I can see everything works as expected. You have a full example where it doesn't work?

bencresty commented 2 years ago

Thanks! I have tested with your glb model and PixiJS 6.4.2 and from what I can see everything works as expected.

Thanks for your response. Interesting. There's nothing I can think of we can do differently if you're using the pixi loader too to load the glb. The only thing I can think of is that I use the defaultQueryString parameter of the loader to prevent browser caching issues (like loader.defaultQueryString = 'h=494o64968'), but again, the exact same configuration worked fine before with all previous pixi versions as described (tested it today again).

Obviously something has changed in pixijs that's causing this to be broken. At least when using webpack, as I do since 2017 and never caused any issues with pixi before. But I've seen that pixi 6.4.0 was having major issues using webpack and they changed things up after it and created 6.4.2. Perhaps something is still a problem while only using webpack?

Are you loading the glb using the plain pixijs loader? Or are you doing something special? Could you please share a snippet of your code to see what you might do differently?

You have a full example where it doesn't work?

I'd like to put some in some online editor, but there's always the issue with with cross domain security issues to load the glb and I don't know a place where I could store the glb online to reach it from such online editor. Projects here are full of settings, configs, build systems, web pack, automation, batch files etc. and all and I don't have the time to re-invent the wheel creating a small project locally to zip at the moment.

I'm just using the pixijs loader to load the glb tho. Nothing fancy about it. Just:

bencresty commented 2 years ago

Just did a quick tests. Findings:

But:

node_modules\pixi3d\dist\pixi3d.js: image

bencresty commented 2 years ago

I don't know how everything is supposed to work, but just simply watching inside node_modules\@pixi\loaders\dist\esm\loaders.js at around line 1429 in the Loader.prototype._add function initialization the options parameter is not set when the glb passes, so it never knows it should be of XHR_RESPONSE_TYPE.BUFFER, so this.resources[name] = new LoaderResource(name, url, options); returns the wrong type.

Looks to me that the add() method of pixi3d gets fired, but for some reason doesn't set the right resource type for some reason, causing the loader to return a resource of xhr 'Text' type instead of arraybuffer.

But I'm in no way an expert on pixijs/pixi3d inner workings. Just doing some quick'n'dirty debug trying. So hopefully this is still helpful.

node_modules\@pixi\loaders\dist\esm\loaders.js image

jnsmalm commented 2 years ago

You can try this:

  1. Download the repo of Pixi3D locally
  2. Copy your test.glb to serve/assets
  3. Change the 2 lines that have "assets/teapot/teapot.gltf" to "assets/test.glb"
  4. npm install
  5. npm run serve
  6. Go to http://localhost:8080/

Does this work?

jnsmalm commented 2 years ago

I can try to create a project using webpack and see if it changes anything. You use both PixiJS and Pixi3D from npm?

bencresty commented 2 years ago

You use both PixiJS and Pixi3D from npm?

Yes

"pixi.js": "6.4.2",
"pixi3d": "^1.4.1",
bencresty commented 2 years ago

You can try this:

  1. Download the repo of Pixi3D locally
  2. Copy your test.glb to serve/assets
  3. Change the 2 lines that have "assets/teapot/teapot.gltf" to "assets/test.glb"
  4. npm install
  5. npm run serve
  6. Go to http://localhost:8080/

Does this work?

NPM and PNPM don't even install. Next to missing peer dependency issues index.ts throws an error causing npm install or pnpm install to fail on the pixi3d and pixi3d.min builds. I'm normally using PNPM, but I also did a clean install with NPM, same story.

image

jnsmalm commented 2 years ago

That's is odd, npm install is used in the same way when building using GitHub Actions. I think this is specific to your machine unfortunately.

jnsmalm commented 2 years ago

Don't even know what pnpm is.

bencresty commented 2 years ago

Don't even know what pnpm is.

PNPM is just like NPM and has all the same commands. But it's using a way smarter system with symlinks under the hood, so if you have a thousand projects with all their node_modules folders they don't all have milions of files inside of them, but point via symlinks to one single 'source of truth', meaning one place where all the real node_modules are stored. This is especially useful on laptops with just a small SSD drive.

But forget about PNPM fot this issue and please keep in mind that eventhough I posted the error message when doing the PNPM install route, the exact same thing happened when fresh installing everything with NPM. Exact same issues.

I think it's a waste of time to debug this tho and rather move back to the original issue. Do you perhaps know a place online where we can store a glb file (for free) to fetch it from an online editor like codepen without cross origin issues?

I'm getting the feeling it has something to do with using Webpack tho as pixi 6.4.0 also had issues with it and you wrote for you it was working with the glb file provided. Did you test with Webpack?

jnsmalm commented 2 years ago

You could try https://codesandbox.io/s/github/jnsmalm/pixi3d-sandbox/tree/master/standard-material, I think you can drag/drop a file there if you are signed in. I didn't have time to Webpack just yet.

jnsmalm commented 2 years ago

Ok, I can reproduce the issue - will let you know when I know more.

jnsmalm commented 2 years ago

Trying adding this to your webpack config file add see if it helps:

resolve: {
    alias: {
      "pixi.js": path.resolve(__dirname, 'node_modules/pixi.js'),
    },
  }
bencresty commented 2 years ago

Trying adding this to your webpack config file add see if it helps:

resolve: {
    alias: {
      "pixi.js": path.resolve(__dirname, 'node_modules/pixi.js'),
    },
  }

With that workaround it indeed works!

lunarraid commented 2 years ago

Unfortunately I think the real answer is going to be upgrading the build to export esm modules. I use create-react-app for my project, and it does not support the above workaround, so I would need to first eject, then change around some configuration files. PixiJS is essentially ending support for UMD modules.

jnsmalm commented 2 years ago

I have started to look into using rollup for build, but it will take a while as I don't think all used loaders exists in rollup.

jnsmalm commented 2 years ago

Hi!

I have now created a branch which uses rollup instead of webpack which hopefully can resolve the issue. The build is now setup to create two different file formats:

@lunarraid @bigtimebuddy Does this setup seem good to you? Both of you seem to have better knowledge regarding these types of formats, so maybe you could help out and see if there is somethiong else that needs to be done? I would be grateful if you could find the time to have a look at the config file rollup.build.js and see if anything seems off.

The commit can be found at https://github.com/jnsmalm/pixi3d/commit/6bc6add652f4d7f8402e2613e33905c2e87407b0

@lunarraid Maybe it's possible to test this branch before I push it to release and see if it fixes the issues?

I have also changed the package.json file to include "exports": "./dist/pixi.mjs" - is that correct? It's still having "main": "dist/pixi3d.js".

@bigtimebuddy Question regarding the pixi3d.mjs:

I tried using this file in a webpage using <script type="module" src="pixi3d.mjs"></script> because it would be nice to be able to to that as well - but it fails.

The first code line of the pixi3d.mjs file is import { Loader, LoaderResource } from '@pixi/loaders'; and the browser doesn't know what @pixi/loaders is. Is this how it's suppose to be, or can it be fixed somehow? Will it only work in a bundler setup?

bencresty commented 2 years ago

@jnsmalm I just see your last comment. Just a quick reaction and request:

Please don't use the mjs-extension and just use js inside a folder like esm. Otherwise everybody should make exceptions in things like webpack loaders / babel etc. and IDE configurations to import / understand only pixi3d with this different extension. I don't even know any other lib that even uses the mjs-extension. So please just keep using js as file extention. Thanks!

[edit] as an example of the output you could see it in the pixi lib itself: inside the dist-folder it has a subfolder called esm containing js files (and their maps)

jnsmalm commented 2 years ago

@bencresty Thanks for the input! Just looked at the release of the latest PixiJS and it looks like this, also uses the mjs extension. But maybe that is not how it should be packaged in npm?

image
bencresty commented 2 years ago

@jnsmalm Looking at an installed pixijs 6.4.2 (so the same version) node package in a project I see here: image

Looks like the urls you have are not for npm use, but are fully pre-compiled versions for direct use in browsers. That's something else

bigtimebuddy commented 2 years ago

@jnsmalm dealing with dependencies is an issue with browser modules. Because there's no node resolution, you need to manually replace the imports with a local or absolute path or use a global name. It's annoying and not that flexible. Pixi's mjs works in the browser because there are no external dependencies.

jnsmalm commented 2 years ago

@bigtimebuddy Thanks for clearing that up.

@bencresty @bigtimebuddy I did some changes after your feedback https://github.com/jnsmalm/pixi3d/commit/f672417706afa05b8dfd183f20170cd1206fe2e4

bencresty commented 2 years ago

@jnsmalm thanks for the quick actions and even learning rollup!

jnsmalm commented 2 years ago

This should hopefully be resolved now, let me know otherwise!

bencresty commented 2 years ago

@jnsmalm Just did a quick upgrade to 1.5.0 and don't see the errors anymore (and without the pixi.js alias workaround). Didn't do an indepth test in browsers and all, but in this quick check I see everything working fine now, so looks like it's fully fixed!

jnsmalm commented 2 years ago

Great :)

bencresty commented 2 years ago

@jnsmalm Sorry, guess I was a little bit to soon. I only did a quick test with webpack-dev-server in between other jobs.

But now that I figured let's finish the change and create a new production build not only is webpack extremely slow while building (but that could be all kinds of things that has changed in the meantime, like nodejs, pnpm etc.)... in the end the webpack build looked stuck but finished the build.

It told me to install all the seperate @pixi stuff tho beforehand so installed all these before.

But now that I'm looking at the output of the project it's throwing an error which wasn't there before (and I didn't change anything on the project since the moment I started this thread as the project is finished): image

So something doesn't seem quite right unfortunately. And it's obviously problematic when running in production environment of webpack, not in the dev mode.

I'm very busy doing other projects now (just thought I could do this quick update in a few minutes and finish it but that didn't seem to be the case), so don't have the time to dive into this further, but wanted to let you know.

jnsmalm commented 2 years ago

Ok, my tests with rollup worked fine. I'll check webpack as well. Are you using the UMD or ESM modules?

bencresty commented 2 years ago

I didn't change the imports. This is how I import, so these should be ES6 modules. (I use babel in this project for source code and some own modules I use via package.json, but don't use babel for this module btw):

import { Model, Light, LightingEnvironment, Camera, PostProcessingSprite } from 'pixi3d';

To underline: it works when using webpack in dev mode with the webpack-dev-server. So it looks like something is 'shaken out of the tree' a little too much by webpack or something to do with compression/optimization.

jnsmalm commented 2 years ago

Are you using PixiJS 6.4.2?

jnsmalm commented 2 years ago

@bencresty Let me know when/if you have time do debug more. I have tested all sorts of combinations and it seems to work with production settings in webpack. One thing I noticed though was due to peerDependencies being added in 1.5.0 those gets downloaded so it's important the webpack uses the correct version of PixiJS. I had to do this in webpack config:

alias: {
  "@pixi/core": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/core"),
  "@pixi/loaders": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/loaders"),
  "@pixi/settings": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/settings"),
  "@pixi/math": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/math"),
  "@pixi/display": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/display"),
  "@pixi/constants": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/constants"),
  "@pixi/utils": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/utils"),
  "@pixi/ticker": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/ticker"),
  "@pixi/sprite": path.resolve(__dirname, "node_modules/pixi.js/node_modules/@pixi/sprite"),
}

@bigtimebuddy Are those peerDependencies really needed in package.json?

bencresty commented 2 years ago

@jnsmalm sorry, I want to help you and thought I could help you by doing that upgrade and see if everything was fine, in between other work, but that was about it I'm afraid. TBH I really have no time following weeks and am working on completely different projects now

jnsmalm commented 2 years ago

@bencresty No problem at all :) With my tests everything seems to work fine. I'll close this one for now and wait if anyone else experienced any issues.

bencresty commented 2 years ago

@jnsmalm I feel a little bad that I can't make more time to help you right now after you helping me. I see that I forgot to mention in my comment a little better the why; it's real busy here rightnow, meaning no-breaks-overtime busy, lot of things happening at once. Doing just a quick install of a version and see if it works I can make time for if that helps, but not for things needing more time, like full debugging, that's what I meant to write. Wanted to be honest about that instead of letting you waiting.

BTW, I see that you're getting great tips by the Goodboys and that probably the inner module usage will change. Great you reached out to them and perhaps that also fixes the error I was seeing? If you want me to tryout a version after that change just let me know