New ambient occlusion example HBAO vs N8AO

I noticed this new example for ambient occlusion…

https://threejs.org/examples/?q=ambient#webgl_postprocessing_hbao

Looks nice, but is there an official comparison of this to N8A0?

Performance/limitations etc?

Why has this been included in the official examples but the N8AO not?

This issue may explain the similarities - it’s just the same mix of algorithms implemented by 2 separate devs.

For what GH history shows, N8AO was never submitted to be included in examples with an external tag - so it was just never added there.

2 Likes

Performance/limitations etc? / Why has this been included in the official examples but the N8AO not?

you would typically use n8ao with GitHub - pmndrs/postprocessing: A post processing library for three.js. which runs faster than jsm-ec as it globs all shaders into a single über shader in a single render. official could mean that someone made a pr, someone else merged it. but n8ao is under actice maintenance and improving, i wouldn’t typically expect this from three/examples because authors loose access and rights. curious if the author of the hbao example gets to carry out the rename for instance.

1 Like

Whilst I appreciate there can be multiple solutions to achieve the same thing, it seems odd that if the post-processing library is so much better than the official one, why not just reference that?Why does three promote a worse solution? I tend to stick with the official things, and I guess new people do too, so it’s just adding confusion (in my opinion).

Would be a bit unfair to call it that way.

  1. I think @drcmda message is a bit more dramatic than necessary. pmndrs/postprocessing is not as significantly better in terms of performance than the built-in one. The core of how they work is the same, it still does the render targets, it still does the separate shader for separate pass (there’s no “one uber shader” created there in realistic cases), it still puts it all together into a frame. In specific cases (ie. tons of effects that can be bundled together in a specific order) it’s better than core threejs postprocessing, in other cases (rendering two separate scenes one-over-another without buffer clearing in-between) it’s not :pray:

  2. The solution included in three/examples isn’t “worse” - but it’s definitely simpler and it’s easier to build your custom things upon it, compared to N8AO repo which has a multi-file structure, build steps, and adds support for additional, miscellaneous stuff (not necessary in some cases.) Similarly to other core postprocessing effects, HBAO one is contained within a single file and can just be copied and overridden. So, going all the way back - it’s not worse in any way, it has a bit of a different purpose :sweat_smile:

For a similar reason three has its own Octree implementation - three-mesh-bvh outperforms it in almost every way, except simplicity. It’d be easier for you to build something yours from core Octree than from the giant codebase that three-mesh-bvh is.

1 Like

well, again, you have a pr that gets merged, meanwhile the original codebase is being maintained. why three does this i don’t know but official doesn’t mean much.

one reason may be that threejs has an anti tooling stance. it doesn’t want people install packages. it would rather take a snapshot of code and never maintain it, than to cherish an eco system where open source authors can prosper. like in this case, you consider it official, now n8 has one user less.

as for postp, jsm/ec renders your whole scene for every pass you give it. postp is under active maintenance, jsm/ec is a snapshot from x years ago and occasionally gets a fix to work with current three.

i know the postp author, he’s close to giving up. this is how dozens of vital threejs libraries have died. ask n8 what he thinks of the new ao effect in examples/jsm for instance.

__
btw mrdoob is pretty open, he wants a healthy eco system, recognizes when something’s better: https://twitter.com/mrdoob/status/1665775500533579781 and looks like at least the examples could point to postp in the future Update post processing examples with pmdrs postprocessing library by AxiomeCG · Pull Request #26247 · mrdoob/three.js · GitHub though i don’t think this will go forward because like i mentioned some of threes maintainers are against externals.

1 Like

@drcmda like you, I think the postprocessing library is a good thing, and I would like to see that thrive. And perhaps even to replace the official THREE.EffectComposer. But your comments are often very demotivating to three.js maintainers — moreso than anyone else’s in this community, I read your comments and want to stop volunteering on the project altogether.

You can of course spend your free time as you choose. But I wish you’d channel this energy into writing up what you actually wish would happen, e.g. as an RFC, rather than taking shots at three.js and its maintainers in comments and tweets.

looks like at least the examples could point to postp in the future… though i don’t think this will go forward because like i mentioned some of threes maintainers are against externals.

No one is against this PR. Ricardo has voiced support. I’ve spent free time working to review it. From my understanding, the author has not had time to complete it, and anyone else in the community would be welcome to volunteer.

8 Likes

Yikes, I didn’t mean to open a can of worms here :grimacing:

I’ve been happy with the results and progress with N8AO, and it doesn’t seem like this new example offers anything over it, which I guess is really what I wanted to know.

A month ago I started a new project Oops.js (Only One-pass Shader). It is in very early stage, so do not rush to use it. When completed it is supposed to do these things with Three.js shaders:

  • all sequential passes that can be merged in a single shader pass will be merged (this reduces the excessive pouring of data between render targets)
  • all constant parameters will be baked into the code (so shader compilers can optimize better)
  • expected performance gain is at least 50% (when merging is possible)
6 Likes

I want to add that the merge of HBAOPass was premature, imo. I’ve written my concerns in HBAO pass is not the "real" HBAO, but N8AO · Issue #27295 · mrdoob/three.js · GitHub and Examples: webgl_postprocessing_ao (SSAO, SAO, N8AO, HBAO, GTAO) by Rabbid76 · Pull Request #27296 · mrdoob/three.js · GitHub and I do vote to remove the HBAOPass from the repository.

No one is against this PR.

I’m okay with adding an external example for the postprocessing package but I’m against replacing EffectComposer and the pass classes. Post processing is an important component for a 3D library and it has to be part of the main repository. The core project needs to define/standardize how passes should be implemented and must provide the component that processes the pass/effect chain in an optimized fashion.

I don’t think EffectComposer is that much worse like claimed by @drcmda. Statements from him like “jsm/ec renders your whole scene for every pass you give it” are just wrong (btw: not the first time he writes misleading nonsense like that). After the feature update in the summer, the last relevant bits that are missing is buffer sharing (meaning you generate e.g. a normal buffer just once and share it in all relevant passes) and uber shader generation, imo. Buffer sharing could be added to the current EffectComposer with no breaking changes. I’ve planned to do this and also have a mental roadmap but just had not the time so far. I would be glad if someone else from the community could help with implementing Allow post effects to request access to depth and normal passes · Issue #7668 · mrdoob/three.js · GitHub. We could chat about the details and I would also review the code as good as possible. Having this in EffectComposer would close the performance gap to postprocessing in certain use cases.

The uber shader generation is cool but I have originally not planned to add it to EffectComposer because of the respective complexity. When we start implementing post processing for WebGPURenderer with TSL and node material, we can implement this feature in a very elegant fashion and also provide WebGPU/WebGL support at the same time which postprocessing does not, afaik.

However, I’m excited to hear about @PavelBoytchev plans and would love to see this in action! Maybe this is something we can also integrate in the main repository :+1: .

To clarify, external projects are fine and I have never claimed they are bad or unfavorable in some fashion. Actually, I am one of the collaborators that often voted to move certain features out of the repository into third party packages so we (the core team) can focus on other stuff. But certain components are ciritical and should be part of the main repository. Post processing is one of it.

10 Likes

I’m okay with adding an external example for the postprocessing package but I’m against replacing EffectComposer and the pass classes. Post processing is an important component for a 3D library and it has to be part of the main repository. The core project needs to define/standardize how passes should be implemented and must provide the component that processes the pass/effect chain in an optimized fashion.

Buffer sharing could be added to the current EffectComposer with no breaking changes. I’ve planned to do this and also have a metal roadmap but just had not the time so far. I would be glad if someone else from the community could help with implementing Allow post effects to request access to depth and normal passes · Issue #7668 · mrdoob/three.js · GitHub . We could chat about the details and I would also review the code as good as possible. Having this in EffectComposer would close the performance gap to postprocessing in certain use cases.

I’d be happy to help champion this. I find myself having to create similar with every project, and I just started a new position and would rather share/outsource it this time. I’ve preferred pipeline-based designs for this from a scheduling standpoint, but maybe there’s a way we can conventionalize common dependencies with the current passes and keep it simple.

I think he just meant that there was a separate render/resolve which is absolutely crippling on mobile even if they were free to perform.

This is something that an uber shader would help with for non-convolution effects (which can be safely merged), but I’d agree that this is much more feasible with the nodes-material builder since descoping different passes is very error prone otherwise (lamina is another example). Makes it a lot easier for more complex user-land effects, including postprocessing, three-gpu-pathtracer, etc.

To clarify, external projects are fine and I have never claimed they are bad or unfavorable in some fashion. Actually, I am one of the collaborators that often voted to move certain features out of the repository into third party packages so we (the core team) can focus on other stuff. But certain components are ciritical and should be part of the main repository. Post processing is one of it.

Thank you for this. I was hesitant to even mirror this in public forks like three-stdlib since I was worried about community response and licensing. I noticed that significant code was copied/pasted verbatim, although there were physical enhancements. Maybe we can direct the author to the sources they referenced who have collaborated before so not all is lost.

1 Like

and why would he do that. or rather why would he not do that, had his package been referenced in some way from threejs examples? it’s not like package download stats can be directly converted into hard currency.

I just want to note that the community seems to have matured. In a not so distant past, certain users in this thread would have been banned from this forum. It’s amazing to see that critique can be given without such dire consequences. After all these are just words, part of a discourse.

I really like the links provided. I think they speak towards what @drcmda is saying.

One just sees “examples” and automatically assumes that these are indeed examples.

Eg:

a thing characteristic of its kind or illustrating a general rule.

But obviously, they are not. They are something completely different, only known to maintainers.

Because no one did a PR with N8AO.

I actually asked N8 about this PR and he said that some chunks of code were similar but not others. Judging by the tone of the conversation with him, it didn’t feel like he was opposed to the PR.

1 Like

FWIW this was our exchange on Discord. Realizing that we neglected a typo, but he followed-up with a review which led to Examples: webgl_postprocessing_hbao - remove cosine-weight the hemisphere by Rabbid76 · Pull Request #27185 · mrdoob/three.js · GitHub. No feathers were ruffled or anything.

1 Like

I think what you are saying there is misleading. No one was banned from this forum because one participated in objective discussions. However, topics were closed or posts were flagged and deleted if persons were offended or the discussion become too heated in general.

The problem is that certain users in the three.js community argue in a toxic and provocative way. The moderators and project members can’t just watch this misbehavior and do nothing. Especially since it takes the fun of Open-Source and drives out talented contributors.

The funny thing is when certain users got banned or their posts got flagged, they portray themselves as victims and then write hate posts in other networks about the “unfair” treatment. This kind of ignorance is scary to be honest. But we can not tolerate this even if means to argue with these people.

I think when we have reviewed the PR ,we should have make sure that sources are not copied. A real external example like the ones for three-bvh-csg, three-gpu-pathtracer, three-mesh-bvh or three-subdivide would have been more appropriate. In this way, the example just imports N8AOPass from the original repository.

1 Like

I don’t want to go into all the personal drama and who copied what and who was there first etc. As mentioned in the screenshot it seems to be a combination from the work of N8, 0beqz and Rabbid. I don’t know who is the original owner of the code but I think that’s what the affected people should clarify personally and not in a public forum. And I totally agree with @Mugen87 that it’s important to stop toxic and provocative behavior.

So back to topic: I wanted to add some “technical” thoughts and continue on two things that were already raised by others:

In some cases, simplicity is important and even needed. Depending on your use case you will either need a full-fledged high-end solution (for example if you write a first-person shooter game) or sometimes it’s better to have something lightweight (for example if you visualize something for e-commerce).

Also, that is sometimes a good option. Depending on what you are doing, less tooling can be beneficial. As a seasoned web-dev I know how painful tooling can become especially because the JS community tends to replace everything (including tooling) after some years. Who still knows Grunt, Bower, Gulp, Webpack etc? What package manager to use? npm, yarn 1, yarn 2, pnpm, turbo? Or anything re-written in Rust? Should we use node, bun, or deno? This can become quite tedious pretty fast, so less tooling can be an advantage in some cases as well.

And it’s the same as I wrote above, in some cases more tooling can be better. So it’s always like a typical consultant answer: “It depends”.

But from my point of view and for my use case it’s a little bit sad that we have to delete the “simple” solution from the repo.

It would be awesome to see your support in this task! Let me share my thoughts about a potential solution for Allow post effects to request access to depth and normal passes · Issue #7668 · mrdoob/three.js · GitHub.

Pass

The generic Pass class gets three new properties. needsDepth, needsNormal and later needsVelocity. It also gets a new inject() method that is called by the composer. The method provides the respective textures for the requested data. So a class like SSAOPass would not generate it’s normal and depth texture by itself anymore, it sets needsDepth and needsNormal to true and then let the respective textures be injected by the composer.

EffectComposer

The composer receives the most changes. It gets a new method anaylze() that is called every time the pass chain is updated. E.g. when addPass() or removePasss() is called. Like the name implies, the method analyzes all passes and checks what data are requested. It saves this information in new internal private variables (e.g. this._depthRequested = true).

Besides, the composer gets new render target properties (next to the read and write buffer) to save the beauty pass with an optional depth texture extension and a normal (and later velocity) pass. The render targets are generated with a new class called ScenePass.

ScenePass

We could update the existing RenderPass but I think it’s better to not update it for backwards compatibility reasons. Instead I would introduce ScenePass which defines the context in which beauty, normal, depth and later velocity data are generated. An instance of this class is create with a scene and a camera and then added to the pass chain right at the beginning. It then generates the data based on the _*requested properties of the composer. At a later point when WebGL 1 is out of the way, we can add support for MRT so all passes (beauty+depth+normal+velocity) are rendered in one go.

Even multiple instances of ScenePass in a chain could be supported. However, the dev has to make sure subsequent instances of ScenePass do not clear the render target data.

There is one special bit in the composer: It has to make sure that the first pass after the scene pass receives the beauty render target as the read buffer. After that, the normal read and write buffers are used in a ping-pong fashion as before. Implementing it like that would save an unnecessary copy from beauty to read.

Migrating

When we migrate a pass like SSAOPass to the new logic, we have to make sure users replace their instance of RenderPass with ScenePass. Apart from that, no further migration tasks should be required. This also allows us to migrate the passes selectively and not all at once.

@CodyJasonBennett What do you think of this?
@PavelBoytchev Since you are actively developing in this area with oops.js right now, what do you think?

2 Likes

I have tried to add an external N8AO example but the line import {Pass as $5Whe3$Pass1} from "postprocessing"; in dist/N8AO.js needs to be removed. Pass should be imported only once from the core. There should be no dependency to postprocessing.

Maybe a separate build file could solve this issue?

BTW: This exactly the issue what I have mentioned in my earlier post New ambient occlusion example HBAO vs N8AO - #10 by Mugen87. Basic post processing components like a abstract Pass class have to be defined once in the core repository. Not in third-party ones.