Support angular projects

[imtiazShakil]

Hi,

I fixed some of the missing type declarations. This library can now be imported into Angular and other similar frameworks.

I had to change Geometry to BufferGeometry in src/parsers/CityObjectParser.js because according to [THREE] (https://discourse.threejs.org/t/three-geometry-will-be-removed-from-core-with-r125/22401) Geometry is no longer renderable.

Thanks.

[imtiazShakil]

Hi @liberostelios , many thanks for the review.

Nevertheless, I think we should ditch CityObjectParser and ObjectMaterialParser altogether. This was an initial effort to provide different parsers for different visualization scenarios (e.g. per city object type or per material). These, all, became obsolete and redundant with the use of custom attributes and shaders in CityJSONWorkerParser.

Unfortunately we can't use CityJSONWorkerParser out of the box because it tries to make an http request to download ParserWorker.js from a relative path eg. (http://host:port/helpers/ParserWorker.js) which is blocked by our backend. From my understanding I think cityjson-threejs-loader as a library should not make such a request because that will force user application to make a change which they might not want to.

Here is a screenshot.

Our main aim is to display an individual cityobject which CityObjectParser is suitable. So what is your recommendation?

Can we keep the deprecated CityObjectParser as long as CityJSONWorkerParser's bug is not solved?

Thanks.

[liberostelios]

Hey @imtiazShakil,

I see your point. Nevertheless, I think you have misidentified the problem.

First of all, the library doesn't make an explicit request itself. This is done on the browser because of the creation of a WebWorker to allow the geometry to be parsed on another thread.

Second, your problem here isn't that the request is blocked. What is happened, rather, is that you receive a 404 stating that /helpers/ParserWorker.js does not exist. The blocking message is showing up after that, because I assume that your backend returns some dummy 404 HTML page as a response. Since the browser was expecting a JavaScript file but your backend returns an HTML page, that's why you get the blocking message.

Why is /helpers/ParserWorker.js missing? I assume it's because you haven't made your bundlers (webpack or whatever else you might be using) to include this file in the build and, therefore, it doesn't end up in your deployment. I've had a similar issue and since I am not a bundler expert either, what I ended up doing was to use parsel.js which seems to be "clever" enough to work out of the box with web workers and bundle them correctly.

Therefore, here is what can be done:

• you can try to make your bundler work with the web worker. I could try to take a look at this one, particularly if that's something that we need to resolve in the library as well.
• you can switch to parcel.js which, according to my experience, works fine with this and should not create any issues for you.
• you can try to make a non-WebWorker version of CityJSONWorkerParser (e.g. a CityJSONParser class) that would, basically, use the ChunkParser (like the current parser class) and create all objects etc, just not through ParserWorker.
• you can keep the obsolete classes, although I am not sure how reliable they are, anymore, and they are considered old and unmaintained code.

I have to admit, the last option might seem the most compelling and easy solution, but it is my least favorite and recommended one. On the other hand, I wouldn't want to make you spend too much time on it if you don't think it's worth it, so if you insist on it I can approve this PR. Keep in mind, though, that I will eventually remove these classes in a future update.

[imtiazShakil]

Hi @liberostelios , Thanks for the explanation.

Why is /helpers/ParserWorker.js missing? I assume it's because you haven't made your bundlers (webpack or whatever else you might be using) to include this file in the build and, therefore, it doesn't end up in your deployment. I've had a similar issue and since I am not a bundler expert either, what I ended up doing was to use parsel.js which seems to be "clever" enough to work out of the box with web workers and bundle them correctly.

I investigated on this issue. What I found is both webpack and parcel.js can handle webworker files (creating a separate package for them), but inorder to achieve that the library has to be bundled. I imported un-bundled cityjson-threejs-loader in my angular project, that's why I was facing this issue.

But after I bundled cityjson-threejs-loader using yarn parcel build src/index.js, I stumbled into other issues

• in the dist folder the type declaration files are missing (eg. .d.ts files)
• after manually copying the type declaration files, I was able to import and transpile cityjson-threejs-loader library in my angular project. But then I failed due to some issues where webpack doesn't understand parcel.require. I forgot the actual error message but it was something like this ERROR Error: Uncaught (in promise): ReferenceError: assignment to undeclared variable parcelRequire

So based on the above experience I think external projects may not able to use CityJSONWorkerParser out of the box.

Finally I followed your following advice.

you can try to make a non-WebWorker version of CityJSONWorkerParser (e.g. a CityJSONParser class) that would, basically, use the ChunkParser (like the current parser class) and create all objects etc, just not through ParserWorker.

Now everything is working when I'm using CityJSONParser. So please review this new class. Let me know if you find any bugs.

Thanks.

[imtiazShakil]

Hello @liberostelios , Hope you are doing fine.

Did you get any time to check on the latest commits?

I would like to hear your feedback, thanks!

[liberostelios]

Hi @imtiazShakil,

I haven't been able to test the new class in practice. It seems to be fine, but I think there is a lot of duplication that we could have avoided there. Since I understand that this was already a lot of effort for you and I understand that the code is already at a state that serves the purpose of your project, I'll merge it as it is right now and make sure to refactor it in the future.

Thank you for your contribution 🙌