Skip to content
This repository was archived by the owner on Mar 7, 2025. It is now read-only.

Various fixes#28

Closed
PreferLinux wants to merge 4 commits intomono:masterfrom
PreferLinux:master
Closed

Various fixes#28
PreferLinux wants to merge 4 commits intomono:masterfrom
PreferLinux:master

Conversation

@PreferLinux
Copy link
Contributor

Sorry that it is just two big commits...

Changes released under the MIT license.

@monoadmin
Copy link

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

2 similar comments
@monoadmin
Copy link

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

@monoadmin
Copy link

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

@Therzok
Copy link
Contributor

Therzok commented Oct 30, 2014

Heya, first of all, can you rebase your branch on top of mono/libgdiplus master to get rid of the merge commits? If the remote name of the mono/libgdiplus repository is upstream, it'll look like this:

git rebase upstream/master
git push --force origin/master

…easureString, fixed GdipDrawImageRect

* text-cairo.c:
MeasureCharacterRanges now counts LF, CR, tab as characters for measuring a CharacterRanges. Also utilises layoutRect.
MeasureString now deals with all alignment, rather than DrawString. Doing so has meant that StringAlignment can now work correctly. Also handles boundingBox better.
DrawString has been changed to work with the changes to MeasureString.

* image.c:
GdipDrawImageRect has been changed so that it doesn't translate a metafile a second time. Previously it drew it at (x, y) and then translated it by (x, y), so the translation has been removed.
 * Added a new drawing instruction / record (METAFILE_RECORD_DIBSTRETCHBLT) which is substantially the same as METAFILE_RECORD_STRETCHDIBITS
 * Corrected parsing of certain values so that they're treated as signed instead of unsigned, including adding a GETS macro to complement the existing GETW but returning a SHORT instead of a WORD
 * Fixed scaling for drawing to a specified size and improved translation at the same time
 * Corrected handling of the DIB BitmapInfoHandler ImageSize field -- it now calculates the value when compression is not used (Compression field = 0 / RGB)

Also changed printf to g_warning in metafile.c and wmfcodec.c so that I could see the messages that were being printed when testing via Mono applications.
@PreferLinux
Copy link
Contributor Author

Sure, done that.

Copy link
Member

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it was translating it a second time. It was actually gdip_metafile_StretchDIBits doing it wrong, but that is what was happening when I commented it out -- although I should probably have removed it. Also, in my changes to do with scaling (in gdip_metafile_play_setup) it became necessary to do the translate there (because it has to translate by x / scaleX and y / scaleY to end out with the right position).

@akoeplinger
Copy link
Member

Please change all usages of spaces for intendation to tabs (only in the changed code, not everywhere).

src/wmfcodec.c Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This should probably not be commented out.

Also, I'm not sure replacing all printf's with g_warning is a good approach, is there a reason why this is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it shouldn't be. I commented it out because it was spamming me before I added METAFILE_RECORD_DIBSTRETCHBLT (all the data of a bitmap!) and I couldn't see the "Unimplemented" message. Fixed in the commit I just did.

I just changed to g_warning because I'm not a C programmer and was debugging from within a Mono application. It was one way to get it to actually show me the messages. I saw it being used in gdip_metafile_play_setup, saw that it showed up for me when the printf's didn't, so replaced printf's in wmfcodec.c and metafile.c with that.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I believe the printf's should show up in the console too, that's interesting.
In order to keep the PR small I'd suggest you add them back so the diff only shows the interesting changes.

Note that I'm not familiar with the libgdiplus code myself and not in a position to properly review this ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly because I didn't use debug builds. Or maybe to do with the MonoDevelop Application Output. But I've changed it back.

Also removed some code I had commented out in text-cairo.c, and uncommented a debugging line that'd found annoying and then forgotten.
@directhex
Copy link
Contributor

build

@directhex
Copy link
Contributor

@akoeplinger
Copy link
Member

Hey :) I've extracted and cleaned up the changes into two PRs: #135 and #137. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants