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.

Sign up for GitHub

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

Clean up enum declarations in OpenEXRCore #1134

Merged
merged 4 commits into from
Aug 30, 2021
Merged

Clean up enum declarations in OpenEXRCore #1134

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d230263
Clean up enum declarations in OpenEXRCore
cary-ilm Aug 26, 2021
2d0b65e
cleanup
cary-ilm Aug 26, 2021
6c90679
cleanup #2
cary-ilm Aug 26, 2021
9b32541
Restore enum names, in addition to typedefs
cary-ilm Aug 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions src/lib/OpenEXRCore/coding.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ internal_coding_update_channel_info (

exr_result_t
internal_encode_free_buffer (
exr_encode_pipeline_t* encode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* sz)
exr_encode_pipeline_t* encode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* sz)
{
void* curbuf = *buf;
size_t cursz = *sz;
Expand All @@ -165,11 +165,11 @@ internal_encode_free_buffer (

exr_result_t
internal_encode_alloc_buffer (
exr_encode_pipeline_t* encode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* cursz,
size_t newsz)
exr_encode_pipeline_t* encode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* cursz,
size_t newsz)
{
void* curbuf = *buf;
if (newsz == 0)
Expand Down Expand Up @@ -218,10 +218,10 @@ internal_encode_alloc_buffer (

exr_result_t
internal_decode_free_buffer (
exr_decode_pipeline_t* decode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* sz)
exr_decode_pipeline_t* decode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* sz)
{
void* curbuf = *buf;
size_t cursz = *sz;
Expand All @@ -247,11 +247,11 @@ internal_decode_free_buffer (

exr_result_t
internal_decode_alloc_buffer (
exr_decode_pipeline_t* decode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* cursz,
size_t newsz)
exr_decode_pipeline_t* decode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* cursz,
size_t newsz)
{
void* curbuf = *buf;
if (!curbuf || *cursz < newsz)
Expand Down
2 changes: 1 addition & 1 deletion src/lib/OpenEXRCore/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ exr_result_t
exr_start_write (
exr_context_t* ctxt,
const char* filename,
enum exr_default_write_mode default_mode,
exr_default_write_mode_t default_mode,
const exr_context_initializer_t* initdata)
{
int rv = EXR_ERR_UNKNOWN;
Expand Down
36 changes: 18 additions & 18 deletions src/lib/OpenEXRCore/internal_coding.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,30 +82,30 @@ exr_result_t internal_validate_next_chunk (
/**************************************/

exr_result_t internal_encode_free_buffer (
exr_encode_pipeline_t* encode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* sz);
exr_encode_pipeline_t* encode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* sz);

exr_result_t internal_encode_alloc_buffer (
exr_encode_pipeline_t* encode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* cursz,
size_t newsz);
exr_encode_pipeline_t* encode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* cursz,
size_t newsz);

exr_result_t internal_decode_free_buffer (
exr_decode_pipeline_t* decode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* sz);
exr_decode_pipeline_t* decode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* sz);

exr_result_t internal_decode_alloc_buffer (
exr_decode_pipeline_t* decode,
enum transcoding_pipeline_buffer_id bufid,
void** buf,
size_t* cursz,
size_t newsz);
exr_decode_pipeline_t* decode,
exr_transcoding_pipeline_buffer_id_t bufid,
void** buf,
size_t* cursz,
size_t newsz);

/**************************************/

Expand Down
4 changes: 2 additions & 2 deletions src/lib/OpenEXRCore/openexr_coding.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ extern "C" {
* (that is, so the implementor knows whether to allocate on which
* device based on the buffer disposition).
*/
enum transcoding_pipeline_buffer_id
typedef enum
{
EXR_TRANSCODE_BUFFER_PACKED,
EXR_TRANSCODE_BUFFER_UNPACKED,
Expand All @@ -28,7 +28,7 @@ enum transcoding_pipeline_buffer_id
EXR_TRANSCODE_BUFFER_SCRATCH2,
EXR_TRANSCODE_BUFFER_PACKED_SAMPLES,
EXR_TRANSCODE_BUFFER_SAMPLES
};
} exr_transcoding_pipeline_buffer_id_t;
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we just break source compatibility here? We're not supposed to do that until 4.0.

To keep back compatibility, we need a typedef to make the old name equivalent to the new.

Unless we are saying that the whole OpenEXRCore library is experimental and not officially part of the public API yet, and therefore ok to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we discussed treating the Core library as being in a sort of WIP RFC mode? @kdt3rd does that sound right?

Copy link
Member Author

Choose a reason for hiding this comment

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

On second thought, it's easy enough to leave the enum names in place. These were inconsistently named (most enums have an associated typedef), and I thought better to straighten everything out as soon as possible, before wider use. So technically this now just adds to the API, so should be OK?

I don't think we should regard OpenEXRCore as WIP RFC, it's beyond that.

Copy link
Contributor

Choose a reason for hiding this comment

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

why would this break compatibility? We haven't changed the layout of anything, just are using the typedef instead of the enum foo, that should be fine?

Copy link
Member Author

Choose a reason for hiding this comment

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

My first commit removed the enum name and replaced it with a typedef, which would lead to a compiler error in application code already using the symbol. But this iteration should be benign since it just adds the typedef.


/** @brief Struct for negotiating buffers when decoding/encoding
* chunks of data.
Expand Down
8 changes: 4 additions & 4 deletions src/lib/OpenEXRCore/openexr_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,13 +353,13 @@ EXR_EXPORT exr_result_t exr_start_read (
const exr_context_initializer_t* ctxtdata);

/** @brief Enum describing how default files are handled during write. */
enum exr_default_write_mode
typedef enum
{
EXR_WRITE_FILE_DIRECTLY =
0, /**< Overwrite filename provided directly, deleted upon error. */
EXR_INTERMEDIATE_TEMP_FILE =
1 /**< Create a temporary file, renaming it upon successful write, leaving original upon error. */
};
1 /**< Create a temporary file, renaming it upon successful write, leaving original upon error */
} exr_default_write_mode_t;

/** @brief Create and initialize a write-only context.
*
Expand Down Expand Up @@ -402,7 +402,7 @@ enum exr_default_write_mode
EXR_EXPORT exr_result_t exr_start_write (
exr_context_t* ctxt,
const char* filename,
enum exr_default_write_mode default_mode,
exr_default_write_mode_t default_mode,
const exr_context_initializer_t* ctxtdata);

/** @brief Create a new context for updating an exr file in place.
Expand Down
4 changes: 2 additions & 2 deletions src/lib/OpenEXRCore/openexr_decode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,13 @@ typedef struct _exr_decode_pipeline
* decoding on a GPU). If `NULL`, will use the allocator from the
* context.
*/
void* (*alloc_fn) (enum transcoding_pipeline_buffer_id, size_t);
void* (*alloc_fn) (exr_transcoding_pipeline_buffer_id_t, size_t);

/** Enable a custom allocator for the different buffers (if
* decoding on a GPU). If `NULL`, will use the allocator from the
* context.
*/
void (*free_fn) (enum transcoding_pipeline_buffer_id, void*);
void (*free_fn) (exr_transcoding_pipeline_buffer_id_t, void*);

/** Function chosen to read chunk data from the context.
*
Expand Down
4 changes: 2 additions & 2 deletions src/lib/OpenEXRCore/openexr_encode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,13 @@ typedef struct _exr_encode_pipeline
* encoding on a GPU). If `NULL`, will use the allocator from the
* context.
*/
void* (*alloc_fn) (enum transcoding_pipeline_buffer_id, size_t);
void* (*alloc_fn) (exr_transcoding_pipeline_buffer_id_t, size_t);

/** Enable a custom allocator for the different buffers (if
* encoding on a GPU). If `NULL`, will use the allocator from the
* context.
*/
void (*free_fn) (enum transcoding_pipeline_buffer_id, void*);
void (*free_fn) (exr_transcoding_pipeline_buffer_id_t, void*);

/** Function chosen based on the output layout of the channels of the part to
* decompress data.
Expand Down
Binary file modified src/lib/OpenEXRCore/openexr_part.h
View file
Open in desktop
Binary file not shown.
4 changes: 2 additions & 2 deletions src/lib/OpenEXRCore/part_attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ exr_result_t
exr_get_attribute_by_index (
exr_const_context_t ctxt,
int part_index,
enum exr_attr_list_access_mode mode,
exr_attr_list_access_mode_t mode,
int32_t idx,
const exr_attribute_t** outattr)
{
Expand Down Expand Up @@ -94,7 +94,7 @@ exr_result_t
exr_get_attribute_list (
exr_const_context_t ctxt,
int part_index,
enum exr_attr_list_access_mode mode,
exr_attr_list_access_mode_t mode,
int32_t* count,
const exr_attribute_t** outlist)
{
Expand Down
4 changes: 2 additions & 2 deletions src/test/OpenEXRCoreTest/write.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ testWriteAttrs (const std::string& tempdir)
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_INVALID_ARGUMENT,
exr_get_attribute_by_index (
outf, partidx, (enum exr_attr_list_access_mode) 42, 15, &attrget));
outf, partidx, (exr_attr_list_access_mode_t) 42, 15, &attrget));
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_ARGUMENT_OUT_OF_RANGE,
exr_get_attribute_by_index (
Expand Down Expand Up @@ -1063,7 +1063,7 @@ testWriteAttrs (const std::string& tempdir)
exr_get_attribute_list (
outf,
partidx,
(enum exr_attr_list_access_mode) 42,
(exr_attr_list_access_mode_t) 42,
&attrcnt,
&attrget));
attrcnt = 1;
Expand Down