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

Promotes next to master branch #1132

Merged
merged 131 commits into from Feb 7, 2020
Merged

Promotes next to master branch #1132

merged 131 commits into from Feb 7, 2020

Conversation

HeroicKatora
Copy link
Member

@HeroicKatora HeroicKatora commented Feb 7, 2020

After this, a release PR for 0.23 will be done and the old version will get a version-0.22 branch (in case we have to do any safety critical bug fixes).

fintelia and others added 30 commits June 5, 2019 16:36
It is an open question whether L2 and L4 should be included, or
conversely whether L1 should be eliminated
Also clarify description of ColorType::Unknown and fix a few warnings
Refactor the ColorType enum
Continue refactoring ColorType
A bunch more ColorType improvements for image:next
Make copy_from return an ImageResult instead of a boolean
Update next with latest changes to master
Unlike the other formats this is not a single acronym, doesn't contain
too many capitals and the website and documentation strongly suggests
this way.
Convert ImageFormat variants to CamelCase
Proper capitalization for decoder struct names
HeroicKatora and others added 26 commits February 1, 2020 19:01
Add missing feature requirement dxt for dds
Moves away from a single, open representation of errors. This considers
a previously identified few issues:

* The new variants are somewhat actionable since they at least identify the
  cause more closely.
* They are extensible as most state is opaque to some degree and key
  enumerations are marked as non-exhaustive.
* They are more precise as the opaque state allows storing specialized
  variants for supported formats while still maintaining a uniform
  interface and permitting extension formats outside the library.
* Handling of 'stringification' is more principled as error values can
  be stored as-is instead of converting them on the spot. This might
  also improve the performance when the conversion is not required.

This is part of a patch series. The commit merely adds the new type and
a rough interface but does not yet convert the existing error instances
and return types.
Emulates the old enum variants with the new type through constructor
methods. Note that some variants have been slightly abused and do not
map cleanly to a single new kind. These should be adjusted over time.

The constructors are private to the crate. While exposing them would
help transition, this also allows us to selectively and incrementally
rework our own error usage by deprecating them and fixing occurring
warnings. In particular, we should expose underlying errors as causes
and so on.

This commit is part of a patch series reworking error types. The next
patch switches the old error type for the new one.
* The format detection is switch to clean variants
* The pnm decoder was the only to utilize NotEnoughData and has been
  switched to a generic format error. It should not be an IO error.
* The external test matches on the renamed unsupported variant
Note that these are structural and thus also asserted on the interior
subtypes. (Unless an unsafe impl were to be added but that should never
happen in any case.)
All `new` constructors accept a detailed underlying description while
the shorthands and alternatives are renamed to `from_*_hint` and
`from_kind` respectively. This is supposed to encourage utilizing a
detailed error representation in dependencies with the new constructor.

Note that the UnsupportedError and LimitError have no `new` constructor
but utilize the naming convention regardless. This is to avoid blocking
the name if we introduce an underlying error representation for them.
These are not modified while storing within the error and should be
faded out over time. Storing the Box<str> saves some space in the
representation with the size of a pointer. Note that DecodingError is
actually the largest struct right now due to it supporting some legacy
representations.
Hide some public interface dependencies on gif and jpeg. Note that frame
oriented functions are now unused and will likely be brought back in a
(slightly less generic) manner that is more uniform over other multi
frame formats as well.
Note that some use within the library itself would use a buffer with
wrong buffer size or disregard width and height errors.
See the nightly feature `public-dependency`, tracking issue: #44663
Add travis script and public dependency reference
Two major reasons:

* This avoids the public dependency on `num_ratio`.
* The internal representation can be changed. This already proved
  necessary once (from u16 rational) and in this manner we are free to
  do it again by adding an inner enum representation.

Note that we already have rounding semantics for the meaning of Delay as
gif rounds to the nearest 10ms when writing frames. Any more or less
precise representation can be made to round to the closest u32 ratio.
With the original available as a millisecond ratio this can easily be
done to the accuracy of nanoseconds, which is the finest available.
Given the prior usage, it does not make much sense to provide an exact
conversion. However, it should be clear that precision is not finite and
the representation is not exactly that of a Duration.

The implementation turned out to be fairly involved as the num-fraction
crate does not provide approximation algorithms for fractions. No fear,
we code them ourselves with extensive testing to cover much more than
the required accuracy tests.
@HeroicKatora HeroicKatora merged commit e50ebed into master Feb 7, 2020
@HeroicKatora
Copy link
Member Author

Travis network failures are far from spurious, it's getting somewhat annoying.. This was the third time or so I had to manually restart a failing task due to it in the last week..

@fintelia
Copy link
Contributor

fintelia commented Feb 7, 2020

Does anywhere in this merge include a rustfmt? If not, we should probably run one now that we don't have two branches around

@HeroicKatora
Copy link
Member Author

I didn't do any. Yes, seems like a good idea to do one. Especially seeing if we can configure rustfmt in such a ways as to avoid the aggressive features so that we can put it on CI.

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.

None yet

8 participants