Opened 10 years ago
Closed 10 years ago
#9126 closed enhancement (fixed)
wxImage: make type typesafe (enum wxBitmapType)
Reported by: | troelsk | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | |
Component: | GUI-all | Version: | |
Keywords: | Cc: | troelsk, vadz | |
Blocked By: | Blocking: | ||
Patch: | yes |
Description
Patch:
- enum wxBitmapType instead of long
- Fixed one bad instance of FindHandler(-1), replaced with FindHandler(wxBITMAP_TYPE_ANY)
- IsOk() instead of Ok()
- wxT() instead of _T()
- Added m_orig_type+GetOrigType(), to remember what LoadFile found
Notes:
Tested on MSW only, sorry. Bound to cause [trivial] compilation errors in the other ports. Unlikely to cause any problems in [correct] user code. Image sample compiles clean without modifications.
Attachments (2)
Change History (10)
comment:1 Changed 10 years ago by troelsk
comment:2 Changed 10 years ago by troelsk
- wxImage::SaveFile(const wxString&) returned true unconditionally.
- Changed GetOrigType() to LastType() and made both LoadFile() and SaveFile() set it
File Added: wximage-type.patch
Changed 10 years ago by troelsk
comment:3 Changed 10 years ago by troelsk
Bound to cause [trivial] compilation errors in the other ports
After another look: not necessarily any problems with the other ports.
Using 'wxBitmapType', not 'enum wxBitmapType'.
File Added: wximage-type.patch
comment:4 Changed 10 years ago by vadz
The 3 different patches in this patch (wxBitmapType, IsOk() and wxT()) really need to be separated... is it still possible for you to submit them separately? Also, I'd rather not apply the last one (wxT) at all, it's a completely gratitious change, both wxT and _T are used throughout wx code and I actually prefer _T in wxMSW.
Also, please add documentation of LastType() (if only to explain to _me_ what is it useful for), adding new public functions without documenting them is a really bad idea.
TIA!
Changed 10 years ago by troelsk
comment:5 Changed 10 years ago by troelsk
is it still possible for you to submit them separately?
Not really. Then I'd need to retype everything. I can offer only to take LastType out, which is painful, it is quite bad to be without it. wxImage is tossing away perfectly good information: If calling wxImage::LoadFile(filename,wxBITMAP_TYPE_ANY) all you are getting is a bool. No way of telling which handler hit jackpot. Hence LastType().
New patch:
wxBitmapType + IsOk() + wxT() but no LastType.
Please apply the original patch last to get LastType too.
LastType documentation:
"wxBitmapType LastType() const
Returns the type of bitmap file found by LoadFile, or specified with SaveFile"
File Added: wximage-type-2.patch
comment:6 Changed 10 years ago by troelsk
both wxT and _T are used throughout wx code
Ideally one of them should go, two is one too many.
(In the meantime, I saw that you've started not using either in cpp files - plain "" instead of wxT("") - this will take some getting used to)
comment:7 Changed 10 years ago by wojdyr
- Component set to GUI-all
- Type set to enhancement
"Unlikely to cause any problems in [correct] user code."
Changing long to enum in a few functions in API obviously will result in many build errors (like pen/brush styles) in user code.
I'm not saying it can't be done, but it would be good to test and fix it also on other ports.
comment:8 Changed 10 years ago by vadz
- Resolution set to fixed
- Status changed from new to closed
Untangled the other changes, updated the docs and committed as r53999.
m_orig_data in wxImageRefData.
File Added: wximage-type.patch