mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
103.03k stars 35.41k forks source link

Feature request: Add console message handler #20619

Closed ThreeDfish closed 4 years ago

ThreeDfish commented 4 years ago

Is your feature request related to a problem? Please describe.

1000's of warning messages on the console increase the memory usage of an app until the app crashes, and duplicate warning messages are not needed. Also, the warning message are generated when a specific device doesn't have a specific capability, even though the functionality of the app works, but will cause the app to crash when running for a length of time because the extra warning console messages use up the memory.

Describe the solution you'd like

I would like to request that all console messages ( log, info, warn, error ) be removed from the code and replaced by a console message handler that provides the option of setting the log level as well as options to remove duplicate messages and an option to pass a function to it for an alternative logging system to allow for logging systems such as Winston that allow for remote logging.

Describe alternatives you've considered

Replacing the built-in console message functions is not a good solution.

Additional context

This is an easy feature add, I could do it myself and submit a PR if I know it will get accepted.

mrdoob commented 4 years ago

1000's of warning messages on the console increase the memory usage of an app until the app crashes, and duplicate warning messages are not needed

What browser is that with? And what warning messages are those?

ThreeDfish commented 4 years ago

Safari 13 on iOS 13 on iPhone 6 for example, which apparently doesn't support some of the features of WebGL

mrdoob commented 4 years ago

Safari 13 on iOS 13 on iPhone 6 for example

What other browsers are you experiencing crashes with?

ThreeDfish commented 4 years ago

The Safari on iOS on iPhone6 is the only one that I have experienced that generates a huge number of console warnings related to un-supported WebGL features. It is the only device we currently test on that doesn't support some of the WebGL features. It is also the only iOS device we test on, as we believe that if it runs well there, it will probably run well on any other iOS 13 or iOS 14 device. However, that does not mean there are not users of the production app that are using it on other devices that may not support all of the WebGL features.

I have experienced crashes in Chrome due to a large number of console messages being generated over time, but in those specific instances, the messages are not from ThreeJS but from messages generated from another library. In my experience it is bad for any app or library to generate a huge number of console messages.

mrdoob commented 4 years ago

Yes, but those are browser issues. Browsers shouldn't crash because of console.log() calls. This is an iOS/iPhone6 issue. Apple is not going to fix it and we have less resources than Apple.

I recommend you do console.log = function () {} when your project runs in that config.

gkjohnson commented 3 years ago

@mrdoob With this last release it's become apparent to me that removing the deprecation warning logs are not always immediately within a users control. Two packages I maintain both depended on Matrix4.getInverse which would log a warning as of v0.123.0. These are warnings that a user cannot address because they are caused by a dependency. It might be nice for this use case if the user had the ability to suppress the three.js logs or provide a custom logging callback until the dependencies are up to date so they can debug their own code and update to the latest version. With #20782 it seems there would be value internally, as well.

Just a new use case for this that I came across.

mrdoob commented 3 years ago

Could you give more details? What are the packages?

gkjohnson commented 3 years ago

The two packages that were impacted by that change were gkjohnson/three-mesh-bvh and NASA-AMMOS/3DTilesRendererJS. Both used Matrix4.getInverse internally and would therefore log warnings when used.

3DTilesRendererJS used it in an update function that needed to be called every frame and also used it for a custom optimized raycast traversal in which it would call getInverse for every tile traversed by the ray. So it would log at least once per frame for update() and possibly dozens for a raycast. three-mesh-bvh similarly called getInverse for every raycast performed and when being used for checking for geometry intersections would call it for every leaf node in the bvh with triangles to checked, so it could log dozens of times per frame or more depending on how it's being used.

I got a few issues in three-mesh-bvh within days of the v0.123.0 release (which was great) but from a users perspective it seems their options would be:

  1. Don't update to v0.123.0 until all dependencies that are using deprecated functions are updated and published.
  2. Tolerate possibly dozens or more extra logs in the console until the dependencies are updated.
  3. Override console.log / console.warn to suppress all logging including the users own logs until dependencies are updated meaning they can't really debug with their own console logs.

Let me know if that's what you were looking for!