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

[pixel] Add warning message on Linux #10638

Conversation

NancyLi1013
Copy link
Contributor

@NancyLi1013 NancyLi1013 commented Apr 1, 2020

Add warning message for installing libglu1-mesa-dev on Ubuntu.
Other changes:

  • Add Homepage and Supports in CONTROL
  • Remove unused functions
  • Add unsupported messages

Related issue #10593

Note: No features need test.

@NancyLi1013 NancyLi1013 added the info:internal This PR or Issue was filed by the vcpkg team. label Apr 1, 2020
@linquize
Copy link

linquize commented Apr 1, 2020

Why do you think that pixel does not support Windows?

@NancyLi1013 NancyLi1013 marked this pull request as ready for review April 1, 2020 05:38
@NancyLi1013
Copy link
Contributor Author

I try to install pixel on Windows but failed.
I also noticed that all the triplets on Windows platform were set as fail in ci.baseline.txt.

pixel:x64-uwp=fail
pixel:x64-windows=fail
pixel:x64-windows-static=fail
pixel:x86-windows=fail

@linquize
Copy link

linquize commented Apr 1, 2020

It works on Windows too, at least Release config.
I tested with this example: https://github.com/dascandy/pixel/blob/master/examples/simple/src/simple.cpp

@linquize
Copy link

linquize commented Apr 1, 2020

Oh, you have to patch one thing for MSVC: add return value in https://github.com/dascandy/pixel/blob/1a3631429da9eeac5208584713f358ef0ab09e51/pixel/src/Window.cpp#L270
Then Windows x64 Release will work.

@NancyLi1013
Copy link
Contributor Author

@linquize
Thanks again for your deep investigation about this port.
I will test this on my local.

@NancyLi1013
Copy link
Contributor Author

/azp run

@NancyLi1013
Copy link
Contributor Author

@linquize
I added the return value but there were still many errors like this:

pixeld.lib(Canvas.cpp.obj) : error LNK2005: "public: static class Pixel::Color const Pixel::Color::AliceBlue" (?AliceBlue@Color@Pixel@@2V12@B) already defined in image_swap.cpp.obj
pixeld.lib(Canvas.cpp.obj) : error LNK2005: "public: static class Pixel::Color const Pixel::Color::AntiqueWhite" (?AntiqueWhite@Color@Pixel@@2V12@B) already defined in image_swap.cpp.obj
pixeld.lib(Canvas.cpp.obj) : error LNK2005: "public: static class Pixel::Color const Pixel::Color::Aqua" (?Aqua@Color@Pixel@@2V12@B) already defined in image_swap.cpp.obj
pixeld.lib(Canvas.cpp.obj) : error LNK2005: "public: static class Pixel::Color const Pixel::Color::Aquamarine" (?Aquamarine@Color@Pixel@@2V12@B) already defined in image_swap.cpp.obj
pixeld.lib(Canvas.cpp.obj) : error LNK2005: "public: static class Pixel::Color const Pixel::Color::Azure" (?Azure@Color@Pixel@@2V12@B) already defined in image_swap.cpp.obj
pixeld.lib(Canvas.cpp.obj) : error LNK2005: "public: static class Pixel::Color const Pixel::Color::Beige" (?Beige@Color@Pixel@@2V12@B) already defined in image_swap.cpp.obj

Except for disabling examples, I can build this port successfuuly.
As for the example you tested, It was added in the latest commit and not in release version.

I also try to update this port to the latest commit, there are also many errors.

It seems that there are a lot of work need to be done in pixel source code.

@linquize
Copy link

linquize commented Apr 2, 2020

The port builds successfully, of course.
But for the library user, the example code linking debug dynamic / debug static / Release static not working
The only working config is Release dynamic

@NancyLi1013
Copy link
Contributor Author

Thanks for your description about this.
Personally, it might be better not to support Windows now.
We can update this port if the source code has implemented these functions.

@PhoebeHui
Copy link
Contributor

I think it's better to report the issue to Upstream instead before we support in vcpkg.

@NancyLi1013
Copy link
Contributor Author

The related issue on upstream dascandy/pixel#10

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed waiting for response labels Apr 22, 2020
@ras0219-msft ras0219-msft merged commit d0531aa into microsoft:master Apr 24, 2020
@ras0219-msft
Copy link
Contributor

LGTM, agree with the course of action (re: notifying upstream)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants