wowserhq / wowser

World of Warcraft in the browser using JavaScript and WebGL
MIT License
240 stars 63 forks source link

Some WMO doodads render incorrectly (possibly load twice) #116

Closed fallenoak closed 8 years ago

fallenoak commented 8 years ago

As caught by @timkurvers, it appears some WMO doodads render incorrectly. In fact, it would appear (based on observation) that some WMO doodads may be loaded and placed twice.

An example is the piston doodad in Tinkertown (in Ironforge). As it animates, there's visible z-fighting. Additionally, as it loads, there's a brief period where it's possible to see just a single instance of the doodad, before a second instance then appears.

screen shot 2016-02-10 at 6 16 36 pm

fallenoak commented 8 years ago

This is, as suspected, the result of the same doodad entry being placed twice. In this particular case, doodad index 2996 (the animated pistons) is referenced by both WMO groups 81 and 82.

As part of a recent PR, I migrated doodad loading to directly load referenced doodads as children of a WMOGroup once the group itself finishes loading. Unfortunately, I naively assumed that a given doodad entry would only be referenced by a single WMOGroup. This is clearly not the case.

By way of background, the reason for the change I made on that aforementioned PR is this: we're pretty close to a point where we want to implement portal culling for performance reasons. To reduce the amount of scene traversal when flagging doodads for portal-culled visibility, my thought was that we could make WMO doodads scene children of their appropriate WMO groups, and simply flag visibility on the entire group. Alas, this plan is now dashed upon the rocks.

As it stands, it looks like the appropriate logic for WMO doodad management is going to look more like this:

  1. Trigger loading based on references in WMO groups. This behavior is what happens currently, and helps prioritize doodad loading (since exterior groups load first).
  2. Establish references between each WMOGroup and each loaded doodad (both directions). This will hopefully minimize the pain of portal culling, since we can do the following:
    • On WMOGroup visibility toggle, traverse the set of doodads referenced by the WMOGroup
    • For each traversed doodad, traverse the set of referenced WMOGroups (that aren't the current group), checking if at least one is visible
    • Update the doodad's visibility accordingly
timkurvers commented 8 years ago

Great find! :+1:

Would it be possible to offload all of this behaviour to DoodadManager, which is already serving a similar purpose for doodads shared between map chunks?

fallenoak commented 8 years ago

I suspect doing so would be more hassle than it's worth.

Some reasoning:

fallenoak commented 8 years ago

Huzzah! Dangerously close to being able to work on portal culling in earnest.

timkurvers commented 8 years ago

:smile: