Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Texpacker #18

Merged
merged 12 commits into from
Dec 22, 2017
Merged

Add Texpacker #18

merged 12 commits into from
Dec 22, 2017

Conversation

silverweed
Copy link
Contributor

@silverweed silverweed commented Dec 20, 2017

(See #12)
Texpacker now works hopefully well. I tried sticking to the rest of the tools' style as much as possible, so it should be pretty familiar to review.

Usage

$ ./texpacker -h
Usage: ./texpacker [flags] <file1> [<file2> ...]
  -noheader
    	Don't write header, output raw PNG
  -o string
    	Output file (- for STDOUT) (default "-")
  -strip string
    	Prefix to strip from file paths for hashing (default <cwd>)

Supported input formats:
    JPEG
    GIF
    PNG

Basically it takes input textures as positional arguments and spits out a binary file with the format [header | PNG image]. It can be told to output just the PNG for debugging purposes.
The -strip flag is used to make the tool usable from any directory: the hashed paths will be relative to the path given to -strip.

The packing algorithm is currently maxrects and uses "github.com/adinfinit/texpack/maxrect" which is MIT-licensed.

Header format

The header looks like this:

ParentTexture Hashed Path [4B]
Entry Count               [4B]
Entry0                    [12B]
...

where Entry is:
Hashed Path [4B]
StartX      [2B]
StartY      [2B]
SizeX       [2B]
SizeY       [2B]

Forgot anything?

Copy link
Member

@Hamcha Hamcha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still using a single file for output while it should do two (packed PNG + metadata). My original doc referred to "Header" meaning "C header file", not header of the actual texture.

go build -o bin/tevasm ./tevasm
go build -o bin/texpacker ./texpacker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the "go get" command on top of both build files, just like tools-extra/build.*


// 8 bytes
type Rect struct {
Start, Size Point
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Size isn't a point, so either rename Point to be less specialized (Vector2?)
Or change what Size is?

noheader := flag.Bool("noheader", false, "Don't write header, output raw PNG")
cwd, err := os.Getwd()
checkErr(err, "Couldn't get working directory")
stripPfx := flag.String("strip", cwd, "Prefix to strip from file paths for hashing")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not call this -prefix? The current name sounds like its a toggle flag

// All positional arguments are input files
inputFiles := flag.Args()

maxBounds := image.Point{1 << 16, 1 << 16} // TODO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot a TODO here, flag?

func ToFileHash(s string) FileHash {
hash.Reset()
_, err := fmt.Fprintf(hash, s)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesnt this use checkErr ?

checkErr(err, "Error converting file path %s to absolute", path)
packer.Add(abspath)
}
if err := packer.Save(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn this use checkErr ?


// writeImageAt adds all pixels of `src` to `dst`, starting from pixel at (`x`,`y`).
// It returns an error if the whole source image couldn't be added to the target.
func writeImageAt(dst *image.RGBA, src image.Image, x, y int) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could instead use image.SubImage and the image/draw functions?

if tryOk {
contextSize = trySize
rects = tryRects
shrunk = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will still cause it to try shrinkY, is that ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably a typo in the function I copy-pasted, it should be shrinkY = false in the shrinkY branch.

@Hamcha Hamcha added the enhancement Missing or better functionality label Dec 20, 2017
- add go get to build scripts
- change Point -> Vector2
- use checkErr where appropriate
- change -strip -> -prefix
- pass max bounds via flags
- use image/draw functions instead of writeImageAt
- fix typo in minimizeFit
@silverweed
Copy link
Contributor Author

silverweed commented Dec 21, 2017

Ok, I should have fixed all issues. Now the tool outputs 2 files: <outfile> and <outfile>.atlas, the latter containing the metadata in the exact same format as described in OP.
I removed the -noheader flag and the possibility to output to stdout, as these would yield ill-defined image/metadata pairs.

outpath := flag.String("o", "-", "Output file (- for STDOUT)")
noheader := flag.Bool("noheader", false, "Don't write header, output raw PNG")
outpath := flag.String("o", "out.png", "Output file")
maxW := flag.Int("maxwidth", 1<<16, "Max width of output texture in pixels")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up for discussion, do we wanna support non-square atlasses? if not, only needs 1 flag ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason to, I think.

}
return out, nil
headerOut, err = os.Create(packer.outfile + ".atlas")
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this returns an error, TexPacker.Save will not close the imageOut

@silverweed
Copy link
Contributor Author

Latest commit should have fixed both issues.

@Hamcha Hamcha merged commit 6728419 into hoverguys:master Dec 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Missing or better functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants