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

add support for transparant png #1288

Merged
merged 2 commits into from Nov 20, 2013
Merged

add support for transparant png #1288

merged 2 commits into from Nov 20, 2013

Conversation

@ryanhc
Copy link
Contributor

ryanhc commented Nov 19, 2013

Modified to use libpng to read .png files. We use stb_image to read all other image files.
This requires updated submodule: servo/rust-png#13

This patch is for:
#1279

@larsbergstrom

This comment has been minimized.

Copy link

larsbergstrom commented on d821140 Nov 19, 2013

You will need to add the update to the submodule (which I merged) by doing:

cd src/support/png/rust-png
git pull origin master
cd ../../../..
git add src/support/png/rust-png

This comment has been minimized.

Copy link

larsbergstrom replied Nov 19, 2013

These changes look good otherwise. And we all appreciate the cleanup of stray whitespace :-)

This comment has been minimized.

Copy link
Owner Author

ryanhc replied Nov 20, 2013

I've just executed the above commands.
Just one quick question: The changes to the submodule's already merged.
In that case, why do I need to run the last git add command? That only indicates
that I have local changes. Correct me if I'm wrong :)

This comment has been minimized.

Copy link

larsbergstrom replied Nov 20, 2013

After running those commands, your git status doesn't show:

# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   ../src/support/png/rust-png

? I'm a bit confused, as I believe the rust-png submodule has not been updated. I've merged the PR (which means it is now at the tip of master in the rust-png repository). But submodules in git are not like svn:external in subversion - they are pinned to a particular change, as identified by the hash associated with that commit. The commands I put in my first comment update the servo repo's pointer to that submodule to pull in the head of the rust-png repo.

@ryanhc
Copy link
Contributor Author

ryanhc commented Nov 20, 2013

cd src/support/png/rust-png
git log

shows that the submodule's updated.

What messages do I need to look for?

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Nov 20, 2013

if you do 'git log' in that directory, you should see:

commit bd3b394e35dee36989e3c80abd9bcc1f99affa27
Merge: a06ce5a b422b09
Author: Jack Moffitt <jack@metajack.im>
Date:   Wed Oct 30 14:56:58 2013 -0700

    Merge pull request #11 from kmcallister/rust-upgrade

    Upgrade Rust

After you do git pull origin master in that directory, you should see:

commit 519c291139813bf6cda192a45745013d543c99fe
Merge: bd3b394 a7d6ec6
Author: Lars Bergstrom <lbergstrom@mozilla.com>
Date:   Tue Nov 19 13:15:28 2013 -0800

    Merge pull request #13 from ryanhc/transparent_png

    add support for transparent png

If you do git status in there, you will see no edits because you haven't made any in that submodule:

[lbergstrom@lbergstrom rust-png]$ git status
# HEAD detached from bd3b394

But if you go up a few dirs, you should see an edit of the submodule in the servo repo:

[lbergstrom@lbergstrom src]$ git status
# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       modified:   support/png/rust-png (new commits)

And then you should need to git add and it should all work.

Or is your git log already showing commit 519c291 as the head? I just tested this on a clean servo directory I pulled down, but maybe you're seeing something different?

@ryanhc
Copy link
Contributor Author

ryanhc commented Nov 20, 2013

Thanks for your kind instruction :)
I've updated this pr to refer to the updated submodule.

@larsbergstrom

This comment has been minimized.

Copy link

larsbergstrom commented on 589530c Nov 20, 2013

r+

This comment has been minimized.

Copy link

larsbergstrom replied Nov 20, 2013

@bors: retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 589530c Nov 20, 2013

saw approval from larsbergstrom
at ryanhc@589530c

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

merging ryanhc/servo/transparent_png = 589530c into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

ryanhc/servo/transparent_png = 589530c merged ok, testing candidate = d049978

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

saw approval from larsbergstrom
at ryanhc@589530c

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

merging ryanhc/servo/transparent_png = 589530c into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

ryanhc/servo/transparent_png = 589530c merged ok, testing candidate = 052a543

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 20, 2013

fast-forwarding master to auto = 052a543

bors-servo pushed a commit that referenced this pull request Nov 20, 2013
Modified to use libpng to read .png files. We use stb_image to read all other image files.
This requires updated submodule: servo/rust-png#13

This patch is for:
#1279
bors-servo pushed a commit that referenced this pull request Nov 20, 2013
Modified to use libpng to read .png files. We use stb_image to read all other image files.
This requires updated submodule: servo/rust-png#13

This patch is for:
#1279
@bors-servo bors-servo closed this Nov 20, 2013
@bors-servo bors-servo merged commit 589530c into servo:master Nov 20, 2013
1 check passed
1 check passed
default all tests passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.