Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1493 closed enhancement (fixed)

[PATCH] Data-driven texture loading

Reported by: myconid Owned by: myconid
Priority: Nice to Have Milestone: Alpha 11
Component: Core engine Keywords: patch texturing
Cc: Patch:

Description

At the moment, actor files only permit a <texture> tag, which allows the loading of a single diffuse texture per actor.

I propose a new system that allows multiple texture <samplers> to be loaded dynamically from the Actor files, thus deprecating the existing <texture> tag.

Under this system, the fixed renderpath looks for a hardcoded sampler named "baseTex", which represents the diffuse texture that <texture> used to provide. There are no other changes to the way the fixed renderpath works.

The real usefulness of this addition, however, becomes apparent in shader mode. An arbitrary number of samplers can be loaded and bound to identifier names, which are automatically exposed in the shader program associated with the Actor's material. This sets up a framework that is better suited for visual effects that require data from samplers, such as normal and specular mapping.

This proposal allows samplers to be loaded in the Actor xml files as follows:

    <samplers>
        <sampler name="baseTex" path="base.png"/>
        <sampler name="normTex" path="norm.png"/>
    </samplers>

The "baseTex" is seen by the fixed renderer as if it is a <texture> tag. In the shaders, these will be bound to:

    uniform sampler2D baseTex;
    uniform sampler2D normTex;

Note that <texture> is deprecated but not removed. Under the hood, it redirects to the generalised <sampler> code.

Attachments (4)

multitexture.diff (15.0 KB ) - added by myconid 12 years ago.
All the above
multitexture2.diff (14.7 KB ) - added by myconid 12 years ago.
Modified patch -- REQUIRES actor.diff.zip
actors.diff.zip (88.8 KB ) - added by myconid 12 years ago.
apply in actors/ directory; updates texture entries
UPDATEACTORS.sh (178 bytes ) - added by myconid 12 years ago.

Download all attachments as: .zip

Change History (16)

by myconid, 12 years ago

Attachment: multitexture.diff added

All the above

comment:1 by Kieran P, 12 years ago

Keywords: patch review added
Milestone: BacklogAlpha 11
Summary: Data-driven texture loading[PATCH] Data-driven texture loading

comment:2 by historic_bruno, 12 years ago

Priority: Should HaveNice to Have

by myconid, 12 years ago

Attachment: multitexture2.diff added

Modified patch -- REQUIRES actor.diff.zip

by myconid, 12 years ago

Attachment: actors.diff.zip added

apply in actors/ directory; updates texture entries

by myconid, 12 years ago

Attachment: UPDATEACTORS.sh added

comment:3 by myconid, 12 years ago

As requested by historic_bruno and Mythos_Ruler, the syntax has been changed to

    <textures>
        <texture name="xx" file="yy.xxx"/>
    </textures>

to make it more intuitive to non-programmers.

The deprecation has also been removed and a script is provided to update the actor files to the new format. actors.diff.zip contains a direct patch of the updated files (to be applied in the actors directory) in case you can't use the script.

comment:4 by historic_bruno, 12 years ago

I applied the patch and it worked with no differences that I noticed.

One of the things that sticks out from reading the code is the term "sampler". As in struct TextureSampler. Was that chosen for want of a better name, or is sampler really the most accurate term? I mean really it's just the original image data represented in memory, if it's going to be sampled there would be some logic to do that (perhaps in the shaders?) When I look at TextureSampler, I'm expecting to see something that samples, but it's only a texture pointer and associated name :)

Minor point: I don't like the abbreviated "baseTex", we might as well spell it out "baseTexture" or shorten it to "base". It's no big deal IMO if we take a few more bytes to spell it out clearly ;)

comment:5 by myconid, 12 years ago

Feel free to refactor the identifier names to anything you think is most appropriate. I prefer to use my free time for coding in functionality than for making cosmetic choices. ;)

Please note that there have been a few minor bugfixes to the code since I released that patch. I've set up a fork on github to keep track of things: https://github.com/myconid/0ad/tree/modelmapping

comment:6 by myconid, 12 years ago

Ok, I've finally had a moment to sift through the code a last time and I think we can go ahead with multitexture2.diff.

(After you apply remember to use the UPDATEACTORS script to get the actor files in the right format, as the actors.diff.zip file is probably out of date by now)

comment:7 by historic_bruno, 12 years ago

We should merge in zoot's Actor Editor changes at the same time (after testing). I want to apply this patch soon, mostly I'm waiting for autobuild to be fixed...

comment:8 by Jonathan Waller, 12 years ago

I took a look through this patch and it all looked good to me. I can't find zoot's actor editor changes though?

comment:9 by Kieran P, 12 years ago

zoots changes are at https://github.com/myconid/0ad/commit/04ccb2fad979a93fd4d1275361d1fc9ed911ff7b

There's only a few changes lines and two new files to copy into place, so it should be fairly easy to test.

comment:10 by Jonathan Waller, 12 years ago

Resolution: fixed
Status: newclosed

In 12183:

Add support for multiple UVs and data driven texture loading. From myconid's patches. Fixes #1493 and fixes #1497.

comment:11 by Jonathan Waller, 12 years ago

In 12184:

Update actor editor for data driven textures. From zoot. Refs #1493.

comment:12 by historic_bruno, 12 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.