Terrain

[scrawl]

I've hit a few blockers so far.

• Land textures and plugin indices
  
  This is a huge mess. Each plugin can define new landmass or edit existing one, but it can only use textures that are also defined in that plugin, because the indices would collide otherwise. To look up a land texture correctly, we need the land texture index and the index of the content file that the Land record came from. I've worked around this for now by encoding plugin index in the ID of the record. However proper editing of LandTexture records is completely ruled out until the ESM format can be fixed, because whenever a plugin edits a landscape it needs to copy all land textures it uses. And editing LandTextures without editing all the landscapes using it is not possible. Unless someone can come up with a workaround I have not thought of?
• Data model for Land records
  
  The current implementation of Land records will not fit in an IdCollection, because of lack of copy constructor. Also, storing in a vector is not a good idea efficiency wise, given the large size of the struct.
  Pretty much all of the CollectionBase interface looks unnecessary for this data type. Would it be a problem to not derive from it and create a custom data container?
• On-demand loading
  
  Land data is quite large, so it is allocated and loaded on demand in OpenMW. This is implemented in the record struct itself. As far as I know everything in OpenCS is loaded in advance, but it would be a good idea to make an exception here. Document-wide features like filtering and verifying do not apply to this data type.
• Presentation
  
  The worldspace widget works by having a set of active cells and showing all objects in these cells. We can simply show a fixed terrain block for each active cell, but that's likely not very pleasant for landscape editing. The terrain component is already designed to show all terrain in the worldspace at once, so why not use that for editing? Either add an option for this in the worldspace widget, or add a separate "landscape" widget?

[Zini]

These are complex issues. I need to think about them for a bit. Will get back to you sometimes tomorrow.

[Zini]

Land textures and plugin indices

Messy. Agreed. We will see if we can fix it after 1.0, but we still need to work with what we got for now.

The idea of encoding the content file index in the OpenCS-internal ID sounds reasonable. LandTextures don't have a real ID AFAIK (but there seems to be a stray mId field in the LandTexture struct that needs to be removed), so we would have to create an OpenCS-internal ID anyway.

I don't see why that would be a problem though. LandTexture records are cheap and therefore having to copy some of them when modifying a Land record doesn't sound too bad.

Data model for Land records

We do need the CollectionBase interface and actually all of this interface's functions will be needed for Land textures. We will still have a table view for this type of record (for monitoring modification status, cloning and such). As for the copy constructor problem, the easiest solution would be to create a custom CSMWorld::Land structs which has a boost::shared_ptr<ESM::Land> data member.

On-demand loading

I was thinking about that a while back. On-demand loading of the data (not the record) seems reasonable. I don't think we can do it asynchronously though (i.e. in a separate thread).

Presentation

The advice I can offer regarding this issue is limited, since I don't know how our terrain component works. I will sum up the situation to make sure we are on the same page.

- We can have several views of the same worldspace. These can be configured to show different things (i.e. one might only show objects and no terrain at all, one may show the cells A and B while another one shows the cells B and C).
- When we start an editing task the in progress changes (i.e. unfinished drag) need to be displayed in all views that show the respective cell.
- At this time these changes must not modify the LAND records, because we can only modify records through the command system and the command will be issued only when the drag operation has finished.

For now I assume the Terrain component manages its own copy of the land data (if it doesn't we need some kind of workaround). The above requirements indicate that we need a per-document Terrain storage that handles (possibly) modified terrain data in the cells currently displayed across all views for this document.
I suggest to add a terrain object (either directly from the terrain component or wrapped in a manager class) to the ViewManager.
The tasks of the Cell objects regarding the terrain would then be limited to inform the terrain object of which cells need to be rendered. Ideally everything working on the Cell would still deal only with the Cell class (the terrain object would kinda turn into an implementation detail of the Cell).

[scrawl]

We do need the CollectionBase interface and actually all of this interface's functions will be needed for Land textures. We will still have a table view for this type of record (for monitoring modification status, cloning and such).

That's what I planned. However, note there are LandTexture records and then we have Land records. The table would be used for the former. I don't see why the latter would need a CollectionBase or table view.

As for the copy constructor problem, the easiest solution would be to create a custom CSMWorld::Land structs which has a boost::shared_ptr<ESM::Land> data member.

But if two CSMWorld::Land objects hold a pointer to the same internal ESM::Land, then we have a broken environment, because they can no longer be modified independently. To me, that sounds like we're saying to the compiler "I'm not actually using the copy constructor, so shut up about it" whereas the proper solution should be to not implement it.

I was thinking about that a while back. On-demand loading of the data (not the record) seems reasonable. I don't think we can do it asynchronously though (i.e. in a separate thread).

I'd need to add workarounds for that then, because all loading is based on worker threads (in the QuadTree based implementation that shows all terrain at once, anyway).

For now I assume the Terrain component manages its own copy of the land data (if it doesn't we need some kind of workaround).

Nope. The land data is retrieved from the ESM records and directly converted into Ogre meshes. Copying land data in the component itself would be problematic. What data do we copy? All of it? At what level of detail? I think this concerns the editor more.
Maybe we can keep an internal history of every modification tick of a drag action. When the drag ends, all of the internal history is merged into one command and added to the command history.
Working directly on the ESM data it would also be easier to connect the terrain objects across views. All that is needed are modification events and then all the views can update on their own.

Anyway, so what about this?

The terrain component is already designed to show all terrain in the worldspace at once, so why not use that for editing? Either add an option for this in the worldspace widget, or add a separate "landscape" widget?

[Zini]

That's what I planned. However, note there are LandTexture records and then we have Land records. The table would be used for the former. I don't see why the latter would need a CollectionBase or table view.

Nope. We will have tables both for Land and LandTextures. As mentioned before we need to have the ability to view the modification status of these records and we may also want to perform additional editing actions on the Land records (like clone or delete).

But if two CSMWorld::Land objects hold a pointer to the same internal ESM::Land, then we have a broken environment,

In what situation would that happen? I am not aware that we are copying anything beyond moving around stuff in the container, copying from base to modified and cloning. For the former the default behaviour is fine and for the later two we can easily create workarounds (do an actual copy, that will somehow have to be implemented on the record level).

Nope. The land data is retrieved from the ESM records and directly converted into Ogre meshes. Copying land data in the component itself would be problematic. What data do we copy? All of it? At what level of detail? I think this concerns the editor more.
Maybe we can keep an internal history of every modification tick of a drag action. When the drag ends, all of the internal history is merged into one command and added to the command history.
Working directly on the ESM data it would also be easier to connect the terrain objects across views. All that is needed are modification events and then all the views can update on their own.

We could have an another data structure that holds only the modifications (I am considering something like this for object editing). But that would have to be integrated into the terrain component somehow.
As mentioned before we can not modify the record data during an incomplete editing operation (drag type). Modifying the records must go through the command system (everything else would break undo) and we can issue the command only when the drag operation is complete (otherwise we would flood the undo stack and it would also kill the ability to cancel an incomplete operation).
Anything that can be modified by a non-immediate user action in the scene window is affected. To my count these would be the heightmap, the textures and the vertex paint.

Anyway, so what about this?

If what I wrote doesn't cover this, then I don't know what you are talking about.

[Zini]

Going back to the modifying the record for non-immediate editing operation issue again: We are thinking too complex. Just make a copy of the record (managed by the terrain manager instance in the ViewManager). If you are concerned about memory consumption, these copys can be made on demand and for every unmodified record the regular record collection is used instead.

[scrawl]

If what I wrote doesn't cover this, then I don't know what you are talking about.

Let me rephrase then.

I am proposing a terrain view that does not show terrain for specific cells, but instead shows terrain for all cells in the worldspace. This would be useful for large-scale modifications (initial heightmap generation, applying of noise to the whole map, etc). The question was whether this feature should go into the worldspace widget as an optional mode, or into a dedicated widget.

[Zini]

I see. No, this feature is not needed, since the scenario you describe is already supported. Open a scene view, set it to show only terrain and select all cells from the worldspace (either from the region map or the cell table) and drag them to the scene view. Done.

[scrawl]

Okay, but implementation wise that would be two completely separate paths. We have a grid-based terrain renderer that can show specific cells (but not batch cells together), and a quadtree-based one that shows everything, but can not hide or show individual cells, because they are possibly batched together.

[Zini]

And the difference between these two would be performance mostly? If that turns out to be an issue, we could detect when the 2nd one is suitable and swap it out on the fly. But let's wait with that until we actually have performance problems.