Skip to content

qoi consistent style pass#356

Merged
treeform merged 4 commits intotreeform:masterfrom
guzba:master
Jan 4, 2022
Merged

qoi consistent style pass#356
treeform merged 4 commits intotreeform:masterfrom
guzba:master

Conversation

@guzba
Copy link
Copy Markdown
Collaborator

@guzba guzba commented Jan 3, 2022

  • morepretty auto-formatting

# allocate a buffer 3/4 the size of the pathological encoding

result.add(qoiSignature)
when cpuEndian == bigEndian:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

no tests, untested. we do not support big endian anywhere else and have not / cannot? ensure this actually is all that is needed. rm.

inc run
if run == 62 or off == qoi.data.high:
result.addUint8(opRun or pred(run))
reset run
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

had to look this up, not worth it

if px == pxPrev:
inc run
if run == 62 or off == qoi.data.high:
result.addUint8(opRun or pred(run))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

had to look this up too, not worth it


result = Qoi(
data: newSeq[ColorRGBA](int width * height),
width: int width,
Copy link
Copy Markdown
Collaborator Author

@guzba guzba Jan 3, 2022

Choose a reason for hiding this comment

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

we by convention append our type conversions to the end

copyMem(result.data[0].addr, qoi.data[0].addr, qoi.data.len * 4)
result.data.toPremultipliedAlpha()

func toQoi*(img: Image; channels: range[3..4]): Qoi =
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

be consistent with png's raw api, do not take in channels without tests (this is not run in any tests)

var
width, height: uint32
channels, colorspace: uint8
block:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

see endian comment below

inc p
of opRun:
run = b0 and 0x3f
else: assert false
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this seems bad, throw for real?

px.r = data.readUint8(p+0)
px.g = data.readUint8(p+1)
px.b = data.readUint8(p+2)
inc(p, 3)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

out of convension we only use inc and dec for +1 and -1, otherwise just use +=

## Decodes data in the QOI file format to an `Image`.
decompressQoi(data).toImage()

proc decodeQoi*(data: seq[uint8]): Image {.inline, raises: [PixieError].} =
Copy link
Copy Markdown
Collaborator Author

@guzba guzba Jan 3, 2022

Choose a reason for hiding this comment

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

gave up on seq[uint8] a long time ago. nim's native representation for bytes is string, see standard lib readFile

Comment thread tests/fuzz_qoi.nim
data[pos] = value
value = rand(255).uint8
data[pos] = value.char
echo &"{i} {pos} {value}"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

important to echo, so if fuzzing crashes we can reproduce the case immediately

@@ -1,5 +1,4 @@
import std/endians, chroma, flatty/binny
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

morepretty does not understand [] imports so we do not use them (support could be added tho, not opposed to them if it is automatic)

@treeform treeform merged commit 8970222 into treeform:master Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants