Page MenuHomeWildfire Games
Differential D2356

Adds BMPBlockWriter to prevent huge memory allocation
Needs ReviewPublic
Actions

Authored by vladislavbelov on Oct 5 2019, 2:45 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Currently implemented only for Windows. Other improvements are planned to be in next patches.

Requires D2357 to remove code duplication and reuse existing code.

Test Plan
  1. Apply the patch and compile the game
  2. Run the game on Windows and make sure that screenshots work without crash
  3. Run the game on other platform and make sure that nothing was changed.

Diff Detail

Event Timeline

vladislavbelov created this revision.Oct 5 2019, 2:45 PM
vladislavbelov added inline comments.
source/lib/tex/tex_bmp.cpp
111

I didn't want to use static_cast here, because of less difference in style for lib. Maybe in future.

Vulcan added a comment.Oct 5 2019, 2:53 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/407/display/redirect

Harbormaster completed remote builds in B9742: Diff 10084.Oct 5 2019, 3:15 PM
Vulcan added a comment.Oct 5 2019, 3:15 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/file/common/real_directory.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"

source/lib/file/common/real_directory.h
|  29| class·RealDirectory·:·public·IFileLoader
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classRealDirectory:' is invalid C code. Use --std or --language to configure the language.

source/lib/file/io/io.h
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"

source/lib/file/io/io.h
|  41| namespace·ERR
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceERR{' is invalid C code. Use --std or --language to configure the language.

source/lib/file/vfs/vfs.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/lib/file/vfs/vfs.h
|  34| namespace·ERR
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceERR{' is invalid C code. Use --std or --language to configure the language.

source/lib/file/common/real_directory.cpp
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"

source/lib/file/vfs/vfs.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/922/display/redirect

vladislavbelov updated this revision to Diff 10085.Oct 5 2019, 3:52 PM
vladislavbelov added a subscriber: elexis.

Fixes @elexis `s notes.

Vulcan added a comment.Oct 5 2019, 3:56 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/923/display/redirect

Harbormaster failed remote builds in B9745: Diff 10085!Oct 5 2019, 3:56 PM
Vulcan added a comment.Oct 5 2019, 3:58 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/408/display/redirect

vladislavbelov updated this revision to Diff 10086.Oct 5 2019, 4:02 PM
Vulcan added a comment.Oct 5 2019, 4:05 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/924/display/redirect

Harbormaster failed remote builds in B9746: Diff 10086!Oct 5 2019, 4:05 PM
Vulcan added a comment.Oct 5 2019, 4:07 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/409/display/redirect

Stan added a subscriber: Stan.Oct 5 2019, 4:10 PM
Stan added inline comments.
source/lib/file/io/io.h
315

Magic Number ?

source/ps/Util.cpp
409

So we drop GLES support for screenshots ? Also, shouldn't the whole function be replaced by a warning or something

vladislavbelov added inline comments.Oct 5 2019, 4:17 PM
source/lib/file/io/io.h
315

Block size from the Parameter class.

source/ps/Util.cpp
409

No, for GLES platforms we use the main WriteBigScreenshot function. Block writing is triggered only for big files.

vladislavbelov updated this revision to Diff 10087.Oct 5 2019, 4:26 PM

Removes unused variables.

Vulcan added a comment.Oct 5 2019, 4:32 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/925/display/redirect

Harbormaster failed remote builds in B9748: Diff 10087!Oct 5 2019, 4:32 PM
Vulcan added a comment.Oct 5 2019, 4:33 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/410/display/redirect

Stan added inline comments.Oct 5 2019, 4:33 PM
source/lib/file/io/io.h
315

Why not use new and delete ?

new char[BUFFER_SIZE] ?

vladislavbelov added inline comments.Oct 5 2019, 4:37 PM
source/lib/file/io/io.h
315

To follow common lib low-level style.

Stan added inline comments.Oct 5 2019, 4:42 PM
source/lib/file/io/io.h
315

Can't you at least cast it directly ?

vladislavbelov updated this revision to Diff 10088.Oct 5 2019, 4:49 PM
Vulcan added a comment.Oct 5 2019, 4:53 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/926/display/redirect

Harbormaster failed remote builds in B9749: Diff 10088!Oct 5 2019, 4:53 PM
Vulcan added a comment.Oct 5 2019, 4:56 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/411/display/redirect

vladislavbelov added a child revision: D2357: Adds ScreenshotWriter to minimise code duplication.Oct 5 2019, 8:30 PM
vladislavbelov edited the summary of this revision. (Show Details)Oct 5 2019, 9:02 PM

Revision Contents
Changeset List

PathSize
source/
lib/
file/
common/
1 line
6 lines
io/
34 lines
vfs/
8 lines
22 lines
tex/
60 lines
59 lines
50 lines
ps/
51 lines
70 lines
136 lines

Diff 10088

View Options

source/lib/file/common/real_directory.h

Loading...
View Options

source/lib/file/common/real_directory.cpp

Loading...
View Options

source/lib/file/io/io.h

Loading...
View Options

source/lib/file/vfs/vfs.h

Loading...
View Options

source/lib/file/vfs/vfs.cpp

Loading...
View Options

source/lib/tex/bmp_helper.h

Loading...
View Options

source/lib/tex/bmp_helper.cpp

Loading...
View Options

source/lib/tex/tex_bmp.cpp

Loading...
View Options

source/ps/BMPBlockWriter.h

Loading...
View Options

source/ps/BMPBlockWriter.cpp

Loading...
View Options

source/ps/Util.cpp

Loading...
Wildfire Games · Phabricator